sweb commented on pull request #9428:
URL: https://github.com/apache/arrow/pull/9428#issuecomment-794523002


   @seddonm1 I have pushed the following adjustment:
   
   ```
          // from functions.rs
           BuiltinScalarFunction::RegexpMatch => |args| match (&args[0], 
&args[1]) {
               (c, ColumnarValue::Scalar(ScalarValue::Utf8(Some(pattern))))
                   if c.data_type() == DataType::Utf8
                       || c.data_type() == DataType::LargeUtf8 =>
               {
                   let arr = match c {
                       ColumnarValue::Array(col) => col.clone(),
                       ColumnarValue::Scalar(_) => c.clone().into_array(1),
                   };
   
                   Ok(ColumnarValue::Array(string_expressions::regexp_match(
                       &arr,
                       pattern.as_str(),
                   )?))
               }
      ...
   ```
   
   This way, I define that the regular expression needs to come from a literal, 
so I do not have to take the first element of the array and hope for the best 
and do not have to check for multiple regular expressions. This comes with a 
change I am not sure is wanted: The string_expressions function `regexp_match` 
has two arguments and not a single slice of ArrayRefs.
   
   I am just proposing this because I am not sure that we want to handle 
multiple regular expressions. If supporting multiple expressions is the way to 
go, I will adjust the kernel the way you implemented regexp_replace. Let me 
know what you think!


----------------------------------------------------------------
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.

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


Reply via email to