sam-1112 commented on code in PR #5215:
URL: https://github.com/apache/datafusion-comet/pull/5215#discussion_r3706212354
##########
native/spark-expr/src/predicate_funcs/rlike.rs:
##########
@@ -225,4 +254,60 @@ mod tests {
let result =
expr.evaluate(&RecordBatch::new_empty(Arc::new(Schema::empty())));
assert!(result.is_err());
}
+
+ #[test]
+ fn test_rlike_utf8_array() {
Review Comment:
Thanks for the review — agreed on all three points.
I've added dictionary coverage for `Dictionary(Int32, Utf8)`,
`Dictionary(Int32, Utf8View)`, and a non-Int32 key case (`Dictionary(Int8,
Utf8)`) so the layout dispatch, `take`, and key-type handling are all
exercised. I also collapsed the three near-identical layout tests into a single
looped test (same shape as `test_rlike_scalar_string_variants`), and added an
all-non-null array case.
One related note: `is_match` no longer has the `is_nullable()` if/else — it
now uses `iter().map(...).collect()` — so the no-nulls test is mainly a
regression check rather than covering a separate branch. Happy to adjust
further if you'd like anything else pinned down.
##########
native/spark-expr/src/predicate_funcs/rlike.rs:
##########
@@ -71,7 +79,14 @@ impl RLike {
})
}
- fn is_match(&self, inputs: &StringArray) -> BooleanArray {
+ /// Match the pre-compiled pattern against a string array of any Arrow
string layout.
+ ///
+ /// Keeps the plan-time compiled [`Regex`] rather than calling Arrow's
+ /// `regexp_is_match(_scalar)`, which recompiles the pattern on every
batch.
+ fn is_match<'a, S>(&'a self, inputs: &'a S) -> BooleanArray
+ where
+ &'a S: StringArrayType<'a>,
+ {
Review Comment:
Good suggestion — I've collapsed `is_match` to the `iter`/`map`/`collect`
form (and switched to `&self`), matching `process_parse_url`. That removes the
`is_nullable()` branch entirely, so the uncovered else path no longer exists.
--
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]
