sam-1112 commented on code in PR #5215:
URL: https://github.com/apache/datafusion-comet/pull/5215#discussion_r3706210905
##########
native/spark-expr/src/predicate_funcs/rlike.rs:
##########
@@ -88,6 +103,15 @@ impl RLike {
}
builder.finish()
}
+
+ fn is_match_array(&self, array: &ArrayRef) -> Result<BooleanArray> {
+ match array.data_type() {
+ DataType::Utf8 => Ok(self.is_match(as_string_array(array)?)),
+ DataType::LargeUtf8 =>
Ok(self.is_match(as_large_string_array(array)?)),
+ DataType::Utf8View =>
Ok(self.is_match(as_string_view_array(array)?)),
+ other => exec_err!("RLike requires string type for input, got
{other:?}"),
Review Comment:
Good point — agreed. A non-string child here means an invalid plan rather
than a user-facing execution error, so I've switched the array path to
`internal_err!` as well to match the scalar branch.
##########
native/spark-expr/src/predicate_funcs/rlike.rs:
##########
@@ -37,6 +40,11 @@ use std::sync::Arc;
/// regular expression engine, which are documented at:
///
/// https://docs.oracle.com/javase/8/docs/api/java/util/regex/Pattern.html
+///
+/// Array matching keeps the plan-time compiled [`Regex`] and loops over Utf8 /
Review Comment:
Agreed on the docs cleanup. I don't have the criterion numbers saved anymore
— I'll remove the strong "benches showed regressions" claim from the struct
docs (or soften it) and keep the compile-per-batch rationale only on
`is_match`. Happy to add a proper bench in a follow-up if useful.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
