alamb commented on code in PR #14449:
URL: https://github.com/apache/datafusion/pull/14449#discussion_r1939845829


##########
datafusion/functions/src/regex/regexpmatch.rs:
##########
@@ -153,33 +152,35 @@ impl ScalarUDFImpl for RegexpMatchFunc {
     }
 }
 
-fn regexp_match_func(args: &[ArrayRef]) -> Result<ArrayRef> {
-    match args[0].data_type() {
-        DataType::Utf8 => regexp_match::<i32>(args),
-        DataType::LargeUtf8 => regexp_match::<i64>(args),
-        other => {
-            internal_err!("Unsupported data type {other:?} for function 
regexp_match")
-        }
-    }
-}
-pub fn regexp_match<T: OffsetSizeTrait>(args: &[ArrayRef]) -> Result<ArrayRef> 
{
+pub fn regexp_match(args: &[ArrayRef]) -> Result<ArrayRef> {

Review Comment:
   I agree technically speaking this is an API change but I also think it is 
small and is ok. I will flag this PR as API change but I think it will be 
minimally disruptive



##########
datafusion/sqllogictest/test_files/string/string_view.slt:
##########
@@ -783,7 +783,7 @@ EXPLAIN SELECT
 FROM test;
 ----
 logical_plan
-01)Projection: regexp_match(CAST(test.column1_utf8view AS Utf8), 
Utf8("^https?://(?:www\.)?([^/]+)/.*$")) AS k
+01)Projection: regexp_match(test.column1_utf8view, 
Utf8View("^https?://(?:www\.)?([^/]+)/.*$")) AS k

Review Comment:
   🎉 



##########
datafusion/sqllogictest/test_files/regexp.slt:
##########
@@ -621,7 +667,7 @@ CREATE TABLE t_stringview AS
 SELECT arrow_cast(str, 'Utf8View') as str, arrow_cast(pattern, 'Utf8View') as 
pattern, arrow_cast(start, 'Int64') as start, arrow_cast(flags, 'Utf8View') as 
flags FROM t;
 
 query I
-SELECT regexp_count(str, '\w') from t;
+SELECT regexp_count(str, '\w') from t_stringview;

Review Comment:
   👍  this looks like a driveby fix



##########
datafusion/sqllogictest/test_files/regexp.slt:
##########
@@ -354,6 +377,29 @@ X
 X
 X
 
+# test string view

Review Comment:
   As discussed in 
https://github.com/apache/datafusion/issues/11911#issuecomment-2631559010 it 
would be great to move these tests into string.slt but we can totally do it as 
a follow on as well
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to