Add regexp_extract func by SKY-ALIN · Pull Request #14282 · apache/datafusion

SKY-ALIN · 2025-01-24T17:30:54Z

Which issue does this PR close?

Closes #14280.

Rationale for this change

Adding more functions

What changes are included in this PR?

The implementation of regexp_extract based on Spark regexp_extract

Are these changes tested?

Yes

Are there any user-facing changes?

No

datafusion/functions/src/regex/regexpextract.rs

Omega359 · 2025-01-25T13:21:39Z

Thanks for your contribution! I've left some comments for your review.

datafusion/functions/src/regex/regexpextract.rs

alamb · 2025-01-27T21:37:25Z

FYI @Omega359 noted this is quite similar to

feat: Add regexp_substr function #14323

rluvaton · 2025-01-28T21:37:35Z

FYI Spark regex is not the same as Rust regex and can have different results

docs/source/user-guide/sql/scalar_functions.md

datafusion/functions/src/regex/regexpextract.rs

Omega359

This is looking good.

I may have missed it but I don't see tests for out of bounds indexes (negative #'s, index larger than match group count).
The docs do not say whether the index is 0-indexed or 1-indexed.
I would love to see a benchmark for this but that absolutely could be added as a followup PR
The returning of Utf8View should be corrected - either just return Utf8/LargeUtf8 or fix the return to actually return utf8view if input is utf8view

My primary concern is there are two PR's for essentially the same functionality. I am not in a position to decide which should be included.

datafusion/functions/src/regex/regexpextract.rs

rluvaton · 2025-03-05T13:51:55Z

datafusion/functions/src/regex/regexpextract.rs

+        &self.signature
+    }
+
+    fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {


Can you implement return_type_from_args with marking nullable true only if the any of the input types are nullable (similiar to this #14841)

Omega359 · 2025-03-21T11:58:59Z

@SKY-ALIN - are you able to look into the above review comments?

Omega359 · 2025-04-02T13:32:27Z

I'll take a look at this later today @SKY-ALIN

Omega359 · 2025-04-02T17:39:55Z

I spent some time today looking at this PR. Here are my thoughts:

This udf closely mirrors the duckdb functionality but it does not mirror the typical regex substring functionality found in postgres, mysql, sql server nor oracle
I do not think that datafusion should have both regexp_extract and regexp_substring functionality as the general consensus has been to try and mirror postgresql with respect to functionality if possible. Thus, if I was to choose to include this type of function it would be the regexp_substr, not regexp_extract. That is not to say that there isn't merit with regexp_extract but it may be best to have it in a repo such as datafusion-functions-extra or similar.

As a note the regexp_substr PR (#14323) is about to expire and the author looks to have submitted it and abandoned it. I think there is a number of ideas that you've implemented in this PR that could be incorporated into a new PR perhaps based on the above mentioned PR?

Just a thought if you are interested - I myself really do appreciate the work you've put into this PR ❤️

Add regexp_extract func

ef87360

github-actions bot added documentation Improvements or additions to documentation sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Jan 24, 2025

Omega359 reviewed Jan 25, 2025

View reviewed changes