Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add regexp_substr function #14323

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

osipovartem
Copy link

@osipovartem osipovartem commented Jan 27, 2025

Which issue does this PR close?

Closes #11946

Rationale for this change

Implementation for
regexp_substr()
regexp_substr()

What changes are included in this PR?

Add regex udf that returns the substring that matches a regular expression within a string.

Are there any user-facing changes?

No user-facing changes since this is a new functionality

@github-actions github-actions bot added documentation Improvements or additions to documentation functions labels Jan 27, 2025
@Omega359
Copy link
Contributor

Is this materially different than #14282 ?

let cleaned_value = if let Some(start) = start {
if start < 1 {
return Err(DataFusionError::from(ArrowError::ComputeError(
"regexp_count() requires start to be 1 based".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"regexp_count() requires start to be 1 based".to_string(),
"regexp_substr() requires start to be 1 based".to_string(),


static DOCUMENTATION: OnceLock<Documentation> = OnceLock::new();

fn get_regexp_substr_doc() -> &'static Documentation {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to use the new doc macro as it's been incorporated into most every udf at this point.

// Planner attempts coercion to the target type starting with the most preferred candidate.
// For example, given input `(Utf8View, Utf8)`, it first tries coercing to `(Utf8, Utf8)`.
// If that fails, it proceeds to `(LargeUtf8, Utf8)`.
TypeSignature::Exact(vec![Utf8, Utf8]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We likely want to include Utf8View into this signature.

@Omega359
Copy link
Contributor

Thank you for your contribution! I have 2 concerns:

  • This may be a duplicate of Add regexp_extract func #14282 (more full featured though) as I think they cover the same functionality (though in a different style and under a different function name)
  • It seems to be based on an old version of regexpcount.rs without the updates and fixes for Utf8View, caching and updated doc style. Caching especially would be nice to have as compiling regex's is very expensive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add additional regexp functions
2 participants