fix: coerce SIMILAR TO operands to a common string type#23704
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #23704 +/- ##
==========================================
- Coverage 80.67% 80.67% -0.01%
==========================================
Files 1088 1088
Lines 367591 367729 +138
Branches 367591 367729 +138
==========================================
+ Hits 296563 296673 +110
- Misses 53374 53386 +12
- Partials 17654 17670 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
SIMILAR TO is planned as a regex binary operator, so normalize its operands with regex coercion in the analyzer. This handles typed NULLs, mixed UTF8 physical types, and dictionary values before physical planning while preserving LIKE dictionary behavior. Replace blind regex kernel downcasts with execution errors for plans that bypass the analyzer, and cover planner and runtime cases including dictionary arrays. Closes apache#22886.
dfdf367 to
54f17f7
Compare
| /// Coerce the value and pattern expressions of a string pattern matching | ||
| /// expression (`LIKE`, `ILIKE` or `SIMILAR TO`) to a common type using | ||
| /// the provided coercion rules. | ||
| fn coerce_like_operands( |
There was a problem hiding this comment.
This is a pure refactor 👍🏻
| ) | ||
| })?; | ||
| let expr = match left_type { | ||
| DataType::Dictionary(_, inner) if *inner == DataType::Utf8 => Box::new(expr), |
There was a problem hiding this comment.
This is pre-existing and so I think it should be addressed separately but this does not handle dictionary encoded needles, i.e.
CREATE TABLE t AS SELECT * FROM (VALUES ('user auth failed')) v(s);
CREATE TABLE p AS SELECT * FROM (VALUES ('(auth|login)')) v(pat);
SELECT arrow_cast(t.s, 'Dictionary(Int32, Utf8)') SIMILAR TO p.pat FROM t CROSS JOIN p;I think this can be handled as it's own issue.
There was a problem hiding this comment.
Thanks for the review!
You're right it's pre-existing — I checked the neighboring behavior: the LIKE kernels handle Dictionary(_, Utf8) needles in the array-array path, but the regex kernels (regex_match_dyn) only handle dictionaries in the scalar fast path. With this PR those paths now return a proper error instead of panicking, but dictionary support itself is indeed a separate concern.
Filed #23709 to track it.
Which issue does this PR close?
Rationale for this change
SIMILAR TOpanics withfailed to downcast arraywhenever its operands end up as arrays of different physical types:Root cause:
SIMILAR TOis planned as a regex binary operator (RegexMatchet al.), whose kernel downcasts both arrays to the left operand's array type. But unlikeLIKEand the regex operators (~,~*, ...), theTypeCoercionanalyzer never coercedExpr::SimilarTooperands — it was listed in the "nothing to coerce" arm ofTypeCoercionRewriter— so any type mismatch reached the kernel un-normalized and hit a blind.expect(). Literal patterns take a scalar fast path, which is why the commoncol SIMILAR TO 'pattern'form never showed this.What changes are included in this PR?
Two commits:
Coerce
SIMILAR TOoperands in the analyzer (datafusion/optimizer/src/analyzer/type_coercion.rs)LIKE-operand coercion intocoerce_like_operands(behavior forLIKE/ILIKEis unchanged, including theDictionary(_, Utf8)special case and error texts).regex_coerciontoExpr::SimilarTo— the same coercion the physical regex operators it is planned into use (expr-common/src/type_coercion/binary.rs:218). ANULLpattern is coerced to a typed NULL and evaluates to NULL (matchingexpr ~ NULL), and mixed string types are unified before planning.type_coercion.slt(both reproducers, NULL variants, and the planning-error path) plus analyzer unit tests.Defense in depth in the regex kernels (
datafusion/physical-expr/src/expressions/binary/kernels.rs).expect("failed to downcast array")inregexp_is_match_flag!/regexp_is_match_flag_scalar!withexec_err!, so any expression path that bypasses the analyzer (e.g. a hand-built plan going straight to physical planning) returns an error instead of panicking. Includes a unit test constructing a mismatchedRegexMatchdirectly.Are these changes tested?
Yes:
type_coercion.slt(issue reproducers,NULL/NOT SIMILAR TO NULL,LargeUtf8×Utf8, incompatible-type planning error)../dev/rust_lint.sh,cargo test -p datafusion-optimizer --lib,cargo test -p datafusion-physical-expr --lib, and the relevant sqllogictest files (type_coercion,strings,scalar,regexp,string) — all green.Are there any user-facing changes?
Only the bug fix: queries that previously panicked now either evaluate correctly (
Utf8View×Utf8etc.) or return NULL / a planning error.LIKE/ILIKEbehavior is unchanged.