This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-23704-5b65e70cbeb9e7a3a069703ab9f49b44d31f85fc in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit c111f0a9ff9e56da416c162257a31d3355461f56 Author: kid <[email protected]> AuthorDate: Tue Jul 21 08:16:23 2026 +0800 fix: coerce SIMILAR TO operands to a common string type (#23704) ## Which issue does this PR close? - Closes #22886 ## Rationale for this change `SIMILAR TO` panics with `failed to downcast array` whenever its operands end up as arrays of different physical types: ```sql SELECT 'a' SIMILAR TO NULL; -- panics during planning (constant folding) 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, 'Utf8View') SIMILAR TO p.pat FROM t CROSS JOIN p; -- panics at runtime ``` Root cause: `SIMILAR TO` is planned as a regex binary operator (`RegexMatch` et al.), whose kernel downcasts both arrays to the left operand's array type. But unlike `LIKE` and the regex operators (`~`, `~*`, ...), the `TypeCoercion` analyzer never coerced `Expr::SimilarTo` operands — it was listed in the "nothing to coerce" arm of `TypeCoercionRewriter` — 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 common `col SIMILAR TO 'pattern'` form never showed this. ## What changes are included in this PR? Two commits: 1. **Coerce `SIMILAR TO` operands in the analyzer** (`datafusion/optimizer/src/analyzer/type_coercion.rs`) - Extract the existing `LIKE`-operand coercion into `coerce_like_operands` (behavior for `LIKE`/`ILIKE` is unchanged, including the `Dictionary(_, Utf8)` special case and error texts). - Apply `regex_coercion` to `Expr::SimilarTo` — the same coercion the physical regex operators it is planned into use (`expr-common/src/type_coercion/binary.rs:218`). A `NULL` pattern is coerced to a typed NULL and evaluates to NULL (matching `expr ~ NULL`), and mixed string types are unified before planning. - Regression coverage in `type_coercion.slt` (both reproducers, NULL variants, and the planning-error path) plus analyzer unit tests. 2. **Defense in depth in the regex kernels** (`datafusion/physical-expr/src/expressions/binary/kernels.rs`) - Replace `.expect("failed to downcast array")` in `regexp_is_match_flag!` / `regexp_is_match_flag_scalar!` with `exec_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 mismatched `RegexMatch` directly. ## Are these changes tested? Yes: - sqllogictest: 7 new cases in `type_coercion.slt` (issue reproducers, `NULL`/`NOT SIMILAR TO NULL`, `LargeUtf8` × `Utf8`, incompatible-type planning error). - Unit tests: analyzer coercion plans (NULL pattern, cross string types, error case) and a kernel-level test proving mismatched arrays now error instead of panic. - Verified `./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` × `Utf8` etc.) or return NULL / a planning error. `LIKE`/`ILIKE` behavior is unchanged. --- datafusion/optimizer/src/analyzer/type_coercion.rs | 193 +++++++++++++++++++-- datafusion/physical-expr/src/expressions/binary.rs | 29 +++- .../src/expressions/binary/kernels.rs | 44 +++-- .../sqllogictest/test_files/type_coercion.slt | 62 ++++++- 4 files changed, 294 insertions(+), 34 deletions(-) diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs b/datafusion/optimizer/src/analyzer/type_coercion.rs index 2503fc8072..afd4e980b5 100644 --- a/datafusion/optimizer/src/analyzer/type_coercion.rs +++ b/datafusion/optimizer/src/analyzer/type_coercion.rs @@ -43,7 +43,7 @@ use datafusion_expr::expr_rewriter::coerce_plan_expr_for_schema; use datafusion_expr::expr_schema::cast_subquery; use datafusion_expr::logical_plan::Subquery; use datafusion_expr::type_coercion::binary::{ - comparison_coercion, like_coercion, type_union_coercion, + comparison_coercion, like_coercion, regex_coercion, type_union_coercion, }; use datafusion_expr::type_coercion::functions::{ UDFCoercionExt, fields_with_udf, value_fields_with_higher_order_udf_and_lambdas, @@ -442,6 +442,38 @@ impl<'a> TypeCoercionRewriter<'a> { Ok(e) } + + /// 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. `LIKE` can preserve a dictionary-encoded + /// value expression, while regex array kernels require both operands to + /// have the same physical string type. + fn coerce_like_operands( + &self, + expr: Expr, + pattern: Expr, + coercion: fn(&DataType, &DataType) -> Option<DataType>, + op_name: &str, + preserve_utf8_dictionary: bool, + ) -> Result<(Box<Expr>, Box<Expr>)> { + let left_type = expr.get_type(self.schema)?; + let right_type = pattern.get_type(self.schema)?; + let coerced_type = coercion(&left_type, &right_type).ok_or_else(|| { + plan_datafusion_err!( + "There isn't a common type to coerce {left_type} and {right_type} in {op_name} expression" + ) + })?; + let expr = match left_type { + DataType::Dictionary(_, inner) + if preserve_utf8_dictionary && *inner == DataType::Utf8 => + { + Box::new(expr) + } + _ => Box::new(expr.cast_to(&coerced_type, self.schema)?), + }; + let pattern = Box::new(pattern.cast_to(&coerced_type, self.schema)?); + Ok((expr, pattern)) + } } impl TreeNodeRewriter for TypeCoercionRewriter<'_> { @@ -588,23 +620,14 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> { escape_char, case_insensitive, }) => { - let left_type = expr.get_type(self.schema)?; - let right_type = pattern.get_type(self.schema)?; - let coerced_type = like_coercion(&left_type, &right_type).ok_or_else(|| { - let op_name = if case_insensitive { - "ILIKE" - } else { - "LIKE" - }; - plan_datafusion_err!( - "There isn't a common type to coerce {left_type} and {right_type} in {op_name} expression" - ) - })?; - let expr = match left_type { - DataType::Dictionary(_, inner) if *inner == DataType::Utf8 => expr, - _ => Box::new(expr.cast_to(&coerced_type, self.schema)?), - }; - let pattern = Box::new(pattern.cast_to(&coerced_type, self.schema)?); + let op_name = if case_insensitive { "ILIKE" } else { "LIKE" }; + let (expr, pattern) = self.coerce_like_operands( + *expr, + *pattern, + like_coercion, + op_name, + true, + )?; Ok(Transformed::yes(Expr::Like(Like::new( negated, expr, @@ -613,6 +636,32 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> { case_insensitive, )))) } + Expr::SimilarTo(Like { + negated, + expr, + pattern, + escape_char, + case_insensitive, + }) => { + // `SIMILAR TO` is planned as a regex operator, so its operands + // must be coerced to a common string type using the same + // coercion rules as the physical regex operators. Otherwise + // mismatched operand types panic during execution. + let (expr, pattern) = self.coerce_like_operands( + *expr, + *pattern, + regex_coercion, + "SIMILAR TO", + false, + )?; + Ok(Transformed::yes(Expr::SimilarTo(Like::new( + negated, + expr, + pattern, + escape_char, + case_insensitive, + )))) + } Expr::BinaryExpr(BinaryExpr { left, op, right }) => { let (left, right) = self.coerce_binary_op(*left, self.schema, op, *right, self.schema)?; @@ -812,7 +861,6 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> { | Expr::Column(_) | Expr::ScalarVariable(_, _) | Expr::Literal(_, _) - | Expr::SimilarTo(_) | Expr::IsNotNull(_) | Expr::IsNull(_) | Expr::Cast(_) @@ -2240,6 +2288,113 @@ mod test { Ok(()) } + #[test] + fn similar_to_for_type_coercion() -> Result<()> { + // similar to : utf8 similar to "abc" + let expr = Box::new(col("a")); + let pattern = Box::new(lit(ScalarValue::new_utf8("abc"))); + let similar_to_expr = + Expr::SimilarTo(Like::new(false, expr, pattern, None, false)); + let empty = empty_with_type(Utf8); + let plan = + LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?); + + assert_analyzed_plan_eq!( + plan, + @r#" + Projection: a SIMILAR TO Utf8("abc") + EmptyRelation: rows=0 + "# + )?; + + // NULL pattern is coerced to a typed NULL instead of panicking + // (https://github.com/apache/datafusion/issues/22886) + let expr = Box::new(col("a")); + let pattern = Box::new(lit(ScalarValue::Null)); + let similar_to_expr = + Expr::SimilarTo(Like::new(false, expr, pattern, None, false)); + let empty = empty_with_type(Utf8); + let plan = + LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?); + + assert_analyzed_plan_eq!( + plan, + @r" + Projection: a SIMILAR TO CAST(NULL AS Utf8) + EmptyRelation: rows=0 + " + )?; + + // Utf8View value and Utf8 pattern are coerced to Utf8View + let expr = Box::new(col("a")); + let pattern = Box::new(lit(ScalarValue::new_utf8("abc"))); + let similar_to_expr = + Expr::SimilarTo(Like::new(false, expr, pattern, None, false)); + let empty = empty_with_type(DataType::Utf8View); + let plan = + LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?); + + assert_analyzed_plan_eq!( + plan, + @r#" + Projection: a SIMILAR TO CAST(Utf8("abc") AS Utf8View) + EmptyRelation: rows=0 + "# + )?; + + // Utf8 value and Utf8View pattern are coerced to Utf8View + let expr = Box::new(col("a")); + let pattern = Box::new(lit(ScalarValue::Utf8View(Some("abc".to_string())))); + let similar_to_expr = + Expr::SimilarTo(Like::new(false, expr, pattern, None, false)); + let empty = empty_with_type(Utf8); + let plan = + LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?); + + assert_analyzed_plan_eq!( + plan, + @r#" + Projection: CAST(a AS Utf8View) SIMILAR TO Utf8View("abc") + EmptyRelation: rows=0 + "# + )?; + + // Dictionary values are coerced to the common regex operand type + let expr = Box::new(col("a")); + let pattern = Box::new(lit(ScalarValue::new_utf8("abc"))); + let similar_to_expr = + Expr::SimilarTo(Like::new(false, expr, pattern, None, false)); + let empty = empty_with_type(DataType::Dictionary( + Box::new(DataType::Int32), + Box::new(Utf8), + )); + let plan = + LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?); + + assert_analyzed_plan_eq!( + plan, + @r#" + Projection: CAST(a AS Utf8) SIMILAR TO Utf8("abc") + EmptyRelation: rows=0 + "# + )?; + + // incompatible types are a planning error, not a panic + let expr = Box::new(col("a")); + let pattern = Box::new(lit(ScalarValue::new_utf8("abc"))); + let similar_to_expr = + Expr::SimilarTo(Like::new(false, expr, pattern, None, false)); + let empty = empty_with_type(DataType::Int64); + let plan = + LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?); + assert_type_coercion_error( + plan, + "There isn't a common type to coerce Int64 and Utf8 in SIMILAR TO expression", + )?; + + Ok(()) + } + #[test] fn unknown_for_type_coercion() -> Result<()> { // unknown diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs index e182bc14fa..a39691674d 100644 --- a/datafusion/physical-expr/src/expressions/binary.rs +++ b/datafusion/physical-expr/src/expressions/binary.rs @@ -1279,7 +1279,7 @@ mod tests { use crate::expressions::{Column, Literal, col, lit, try_cast}; use datafusion_expr::lit as expr_lit; - use datafusion_common::plan_datafusion_err; + use datafusion_common::{assert_contains, plan_datafusion_err}; use datafusion_physical_expr_common::physical_expr::fmt_sql; use crate::planner::logical2physical; @@ -3331,6 +3331,33 @@ mod tests { Ok(()) } + #[test] + fn regex_mismatched_array_types_error() -> Result<()> { + // The analyzer coerces both operands of a regex operator to a common + // string type, but an expression that bypasses it (e.g. constructed + // directly) must return an error instead of panicking + // (https://github.com/apache/datafusion/issues/22886) + let schema = Schema::new(vec![ + Field::new("a", DataType::Utf8View, true), + Field::new("b", DataType::Utf8, true), + ]); + let a = Arc::new(StringViewArray::from(vec!["user auth failed"])) as ArrayRef; + let b = Arc::new(StringArray::from(vec!["(auth|login)"])) as ArrayRef; + + // construct the expression directly, without coercion + let expr = binary( + col("a", &schema)?, + Operator::RegexMatch, + col("b", &schema)?, + &schema, + )?; + let batch = RecordBatch::try_new(Arc::new(schema), vec![a, b])?; + let err = expr.evaluate(&batch).unwrap_err(); + assert_contains!(err.to_string(), "failed to downcast array"); + + Ok(()) + } + #[test] fn or_with_nulls_op() -> Result<()> { let schema = Schema::new(vec![ diff --git a/datafusion/physical-expr/src/expressions/binary/kernels.rs b/datafusion/physical-expr/src/expressions/binary/kernels.rs index 39e9c40dbd..fca824c14b 100644 --- a/datafusion/physical-expr/src/expressions/binary/kernels.rs +++ b/datafusion/physical-expr/src/expressions/binary/kernels.rs @@ -27,7 +27,7 @@ use arrow::compute::kernels::boolean::not; use arrow::compute::kernels::comparison::{regexp_is_match, regexp_is_match_scalar}; use arrow::datatypes::DataType; use datafusion_common::{Result, ScalarValue}; -use datafusion_common::{internal_err, plan_err}; +use datafusion_common::{exec_err, internal_err, plan_err}; use std::sync::Arc; @@ -162,14 +162,27 @@ create_left_integral_dyn_scalar_kernel!( /// Invoke a compute kernel on a pair of binary data arrays with flags macro_rules! regexp_is_match_flag { ($LEFT:expr, $RIGHT:expr, $ARRAYTYPE:ident, $NOT:expr, $FLAG:expr) => {{ - let ll = $LEFT - .as_any() - .downcast_ref::<$ARRAYTYPE>() - .expect("failed to downcast array"); - let rr = $RIGHT - .as_any() - .downcast_ref::<$ARRAYTYPE>() - .expect("failed to downcast array"); + // The analyzer coerces both operands to a common string type, but + // expressions that bypass it may still reach here with mismatched + // types, which must surface as an error rather than a panic. + let ll = match $LEFT.as_any().downcast_ref::<$ARRAYTYPE>() { + Some(ll) => ll, + None => { + return exec_err!( + "failed to downcast array to {} for operation 'regex_match_dyn'", + stringify!($ARRAYTYPE) + ); + } + }; + let rr = match $RIGHT.as_any().downcast_ref::<$ARRAYTYPE>() { + Some(rr) => rr, + None => { + return exec_err!( + "failed to downcast array to {} for operation 'regex_match_dyn'", + stringify!($ARRAYTYPE) + ); + } + }; let flag = if $FLAG { Some($ARRAYTYPE::from(vec!["i"; ll.len()])) @@ -210,10 +223,15 @@ pub(crate) fn regex_match_dyn( /// Invoke a compute kernel on a data array and a scalar value with flag macro_rules! regexp_is_match_flag_scalar { ($LEFT:expr, $RIGHT:expr, $ARRAYTYPE:ident, $NOT:expr, $FLAG:expr) => {{ - let ll = $LEFT - .as_any() - .downcast_ref::<$ARRAYTYPE>() - .expect("failed to downcast array"); + let ll = match $LEFT.as_any().downcast_ref::<$ARRAYTYPE>() { + Some(ll) => ll, + None => { + return Some(exec_err!( + "failed to downcast array to {} for operation 'regex_match_dyn_scalar'", + stringify!($ARRAYTYPE) + )); + } + }; if let Some(Some(string_value)) = $RIGHT.try_as_str() { let flag = $FLAG.then_some("i"); diff --git a/datafusion/sqllogictest/test_files/type_coercion.slt b/datafusion/sqllogictest/test_files/type_coercion.slt index 7039e66b38..7ec0f5f1db 100644 --- a/datafusion/sqllogictest/test_files/type_coercion.slt +++ b/datafusion/sqllogictest/test_files/type_coercion.slt @@ -301,4 +301,64 @@ query error does not support zero arguments SELECT * FROM (SELECT 1) WHERE CAST(STARTS_WITH() AS STRING) = 'x'; query error does not support zero arguments -SELECT * FROM (SELECT 1) WHERE TRY_CAST(STARTS_WITH() AS INT) = 1; \ No newline at end of file +SELECT * FROM (SELECT 1) WHERE TRY_CAST(STARTS_WITH() AS INT) = 1; + +################################################################### +## SIMILAR TO type coercion (https://github.com/apache/datafusion/issues/22886) +################################################################### + +# NULL pattern is coerced to a typed NULL and evaluates to NULL instead of panicking +query B +SELECT 'a' SIMILAR TO NULL; +---- +NULL + +query B +SELECT NULL SIMILAR TO NULL; +---- +NULL + +query B +SELECT 'a' NOT SIMILAR TO NULL; +---- +NULL + +# operands of different string types are coerced to a common type +statement ok +CREATE TABLE t AS SELECT * FROM (VALUES ('user auth failed')) v(s); + +statement ok +CREATE TABLE p AS SELECT * FROM (VALUES ('(auth|login)')) v(pat); + +# Utf8View value with a non-scalar Utf8 pattern (issue repro) +query B +SELECT arrow_cast(t.s, 'Utf8View') SIMILAR TO p.pat FROM t CROSS JOIN p; +---- +true + +# LargeUtf8 value with a non-scalar Utf8 pattern +query B +SELECT arrow_cast(t.s, 'LargeUtf8') SIMILAR TO p.pat FROM t CROSS JOIN p; +---- +true + +# Dictionary value with a non-scalar Utf8 pattern must be unpacked before +# reaching the regex array kernel +query B +SELECT arrow_cast(t.s, 'Dictionary(Int32, Utf8)') SIMILAR TO p.pat FROM t CROSS JOIN p; +---- +true + +statement ok +DROP TABLE t; + +statement ok +DROP TABLE p; + +# incompatible operand types are a planning error, not a panic +query error There isn't a common type to coerce Int64 and Utf8 in SIMILAR TO expression +SELECT 1 SIMILAR TO 'a'; + +# a non-string pattern is rejected even earlier, during SQL planning +query error Invalid pattern in SIMILAR TO expression +SELECT 'a' SIMILAR TO 1; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
