tlm365 commented on code in PR #6376:
URL: https://github.com/apache/arrow-rs/pull/6376#discussion_r1753032114


##########
arrow-string/src/regexp.rs:
##########
@@ -107,18 +116,22 @@ pub fn regexp_is_match_utf8<OffsetSize: OffsetSizeTrait>(
             .nulls(nulls)
             .build_unchecked()
     };
+
     Ok(BooleanArray::from(data))
 }
 
-/// Perform SQL `array ~ regex_array` operation on [`StringArray`] /
-/// [`LargeStringArray`] and a scalar.
+/// Perform SQL `array ~ regex_array` operation on
+/// [`StringArray`] / [`LargeStringArray`] / [`StringViewArray`] and a scalar.
 ///
 /// See the documentation on [`regexp_is_match_utf8`] for more details.
-pub fn regexp_is_match_utf8_scalar<OffsetSize: OffsetSizeTrait>(
-    array: &GenericStringArray<OffsetSize>,
+pub fn regexp_is_match_utf8_scalar<'a, S>(

Review Comment:
   > TLDR is I think if we introduced a new function like the following:
   
   @alamb Sounds good 👍 But why do we use `&dyn Array` for the new 
`regex_is_match` function instead of keeping the current implementation?
   
   Or am I misunderstanding you? I understand that we will provide a new 
`regex_is_match` function, and mark the current `regex_is_match_utf8` function 
as:
   ```rust
   #[deprecated(since="54.0.0", note="please use `regex_is_match` instead")]
   pub fn regexp_is_match_utf8(...) { ... }
   ```
   Is that right? 🤔



-- 
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...@arrow.apache.org

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

Reply via email to