Skip to content

Commit 54f17f7

Browse files
committed
fix: coerce SIMILAR TO operands to a common string type
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 #22886.
1 parent 67947b6 commit 54f17f7

4 files changed

Lines changed: 294 additions & 34 deletions

File tree

datafusion/optimizer/src/analyzer/type_coercion.rs

Lines changed: 174 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use datafusion_expr::expr_rewriter::coerce_plan_expr_for_schema;
4343
use datafusion_expr::expr_schema::cast_subquery;
4444
use datafusion_expr::logical_plan::Subquery;
4545
use datafusion_expr::type_coercion::binary::{
46-
comparison_coercion, like_coercion, type_union_coercion,
46+
comparison_coercion, like_coercion, regex_coercion, type_union_coercion,
4747
};
4848
use datafusion_expr::type_coercion::functions::{
4949
UDFCoercionExt, fields_with_udf, value_fields_with_higher_order_udf_and_lambdas,
@@ -442,6 +442,38 @@ impl<'a> TypeCoercionRewriter<'a> {
442442

443443
Ok(e)
444444
}
445+
446+
/// Coerce the value and pattern expressions of a string pattern matching
447+
/// expression (`LIKE`, `ILIKE` or `SIMILAR TO`) to a common type using
448+
/// the provided coercion rules. `LIKE` can preserve a dictionary-encoded
449+
/// value expression, while regex array kernels require both operands to
450+
/// have the same physical string type.
451+
fn coerce_like_operands(
452+
&self,
453+
expr: Expr,
454+
pattern: Expr,
455+
coercion: fn(&DataType, &DataType) -> Option<DataType>,
456+
op_name: &str,
457+
preserve_utf8_dictionary: bool,
458+
) -> Result<(Box<Expr>, Box<Expr>)> {
459+
let left_type = expr.get_type(self.schema)?;
460+
let right_type = pattern.get_type(self.schema)?;
461+
let coerced_type = coercion(&left_type, &right_type).ok_or_else(|| {
462+
plan_datafusion_err!(
463+
"There isn't a common type to coerce {left_type} and {right_type} in {op_name} expression"
464+
)
465+
})?;
466+
let expr = match left_type {
467+
DataType::Dictionary(_, inner)
468+
if preserve_utf8_dictionary && *inner == DataType::Utf8 =>
469+
{
470+
Box::new(expr)
471+
}
472+
_ => Box::new(expr.cast_to(&coerced_type, self.schema)?),
473+
};
474+
let pattern = Box::new(pattern.cast_to(&coerced_type, self.schema)?);
475+
Ok((expr, pattern))
476+
}
445477
}
446478

447479
impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
@@ -588,23 +620,14 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
588620
escape_char,
589621
case_insensitive,
590622
}) => {
591-
let left_type = expr.get_type(self.schema)?;
592-
let right_type = pattern.get_type(self.schema)?;
593-
let coerced_type = like_coercion(&left_type, &right_type).ok_or_else(|| {
594-
let op_name = if case_insensitive {
595-
"ILIKE"
596-
} else {
597-
"LIKE"
598-
};
599-
plan_datafusion_err!(
600-
"There isn't a common type to coerce {left_type} and {right_type} in {op_name} expression"
601-
)
602-
})?;
603-
let expr = match left_type {
604-
DataType::Dictionary(_, inner) if *inner == DataType::Utf8 => expr,
605-
_ => Box::new(expr.cast_to(&coerced_type, self.schema)?),
606-
};
607-
let pattern = Box::new(pattern.cast_to(&coerced_type, self.schema)?);
623+
let op_name = if case_insensitive { "ILIKE" } else { "LIKE" };
624+
let (expr, pattern) = self.coerce_like_operands(
625+
*expr,
626+
*pattern,
627+
like_coercion,
628+
op_name,
629+
true,
630+
)?;
608631
Ok(Transformed::yes(Expr::Like(Like::new(
609632
negated,
610633
expr,
@@ -613,6 +636,32 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
613636
case_insensitive,
614637
))))
615638
}
639+
Expr::SimilarTo(Like {
640+
negated,
641+
expr,
642+
pattern,
643+
escape_char,
644+
case_insensitive,
645+
}) => {
646+
// `SIMILAR TO` is planned as a regex operator, so its operands
647+
// must be coerced to a common string type using the same
648+
// coercion rules as the physical regex operators. Otherwise
649+
// mismatched operand types panic during execution.
650+
let (expr, pattern) = self.coerce_like_operands(
651+
*expr,
652+
*pattern,
653+
regex_coercion,
654+
"SIMILAR TO",
655+
false,
656+
)?;
657+
Ok(Transformed::yes(Expr::SimilarTo(Like::new(
658+
negated,
659+
expr,
660+
pattern,
661+
escape_char,
662+
case_insensitive,
663+
))))
664+
}
616665
Expr::BinaryExpr(BinaryExpr { left, op, right }) => {
617666
let (left, right) =
618667
self.coerce_binary_op(*left, self.schema, op, *right, self.schema)?;
@@ -812,7 +861,6 @@ impl TreeNodeRewriter for TypeCoercionRewriter<'_> {
812861
| Expr::Column(_)
813862
| Expr::ScalarVariable(_, _)
814863
| Expr::Literal(_, _)
815-
| Expr::SimilarTo(_)
816864
| Expr::IsNotNull(_)
817865
| Expr::IsNull(_)
818866
| Expr::Cast(_)
@@ -2240,6 +2288,113 @@ mod test {
22402288
Ok(())
22412289
}
22422290

2291+
#[test]
2292+
fn similar_to_for_type_coercion() -> Result<()> {
2293+
// similar to : utf8 similar to "abc"
2294+
let expr = Box::new(col("a"));
2295+
let pattern = Box::new(lit(ScalarValue::new_utf8("abc")));
2296+
let similar_to_expr =
2297+
Expr::SimilarTo(Like::new(false, expr, pattern, None, false));
2298+
let empty = empty_with_type(Utf8);
2299+
let plan =
2300+
LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?);
2301+
2302+
assert_analyzed_plan_eq!(
2303+
plan,
2304+
@r#"
2305+
Projection: a SIMILAR TO Utf8("abc")
2306+
EmptyRelation: rows=0
2307+
"#
2308+
)?;
2309+
2310+
// NULL pattern is coerced to a typed NULL instead of panicking
2311+
// (https://github.com/apache/datafusion/issues/22886)
2312+
let expr = Box::new(col("a"));
2313+
let pattern = Box::new(lit(ScalarValue::Null));
2314+
let similar_to_expr =
2315+
Expr::SimilarTo(Like::new(false, expr, pattern, None, false));
2316+
let empty = empty_with_type(Utf8);
2317+
let plan =
2318+
LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?);
2319+
2320+
assert_analyzed_plan_eq!(
2321+
plan,
2322+
@r"
2323+
Projection: a SIMILAR TO CAST(NULL AS Utf8)
2324+
EmptyRelation: rows=0
2325+
"
2326+
)?;
2327+
2328+
// Utf8View value and Utf8 pattern are coerced to Utf8View
2329+
let expr = Box::new(col("a"));
2330+
let pattern = Box::new(lit(ScalarValue::new_utf8("abc")));
2331+
let similar_to_expr =
2332+
Expr::SimilarTo(Like::new(false, expr, pattern, None, false));
2333+
let empty = empty_with_type(DataType::Utf8View);
2334+
let plan =
2335+
LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?);
2336+
2337+
assert_analyzed_plan_eq!(
2338+
plan,
2339+
@r#"
2340+
Projection: a SIMILAR TO CAST(Utf8("abc") AS Utf8View)
2341+
EmptyRelation: rows=0
2342+
"#
2343+
)?;
2344+
2345+
// Utf8 value and Utf8View pattern are coerced to Utf8View
2346+
let expr = Box::new(col("a"));
2347+
let pattern = Box::new(lit(ScalarValue::Utf8View(Some("abc".to_string()))));
2348+
let similar_to_expr =
2349+
Expr::SimilarTo(Like::new(false, expr, pattern, None, false));
2350+
let empty = empty_with_type(Utf8);
2351+
let plan =
2352+
LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?);
2353+
2354+
assert_analyzed_plan_eq!(
2355+
plan,
2356+
@r#"
2357+
Projection: CAST(a AS Utf8View) SIMILAR TO Utf8View("abc")
2358+
EmptyRelation: rows=0
2359+
"#
2360+
)?;
2361+
2362+
// Dictionary values are coerced to the common regex operand type
2363+
let expr = Box::new(col("a"));
2364+
let pattern = Box::new(lit(ScalarValue::new_utf8("abc")));
2365+
let similar_to_expr =
2366+
Expr::SimilarTo(Like::new(false, expr, pattern, None, false));
2367+
let empty = empty_with_type(DataType::Dictionary(
2368+
Box::new(DataType::Int32),
2369+
Box::new(Utf8),
2370+
));
2371+
let plan =
2372+
LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?);
2373+
2374+
assert_analyzed_plan_eq!(
2375+
plan,
2376+
@r#"
2377+
Projection: CAST(a AS Utf8) SIMILAR TO Utf8("abc")
2378+
EmptyRelation: rows=0
2379+
"#
2380+
)?;
2381+
2382+
// incompatible types are a planning error, not a panic
2383+
let expr = Box::new(col("a"));
2384+
let pattern = Box::new(lit(ScalarValue::new_utf8("abc")));
2385+
let similar_to_expr =
2386+
Expr::SimilarTo(Like::new(false, expr, pattern, None, false));
2387+
let empty = empty_with_type(DataType::Int64);
2388+
let plan =
2389+
LogicalPlan::Projection(Projection::try_new(vec![similar_to_expr], empty)?);
2390+
assert_type_coercion_error(
2391+
plan,
2392+
"There isn't a common type to coerce Int64 and Utf8 in SIMILAR TO expression",
2393+
)?;
2394+
2395+
Ok(())
2396+
}
2397+
22432398
#[test]
22442399
fn unknown_for_type_coercion() -> Result<()> {
22452400
// unknown

datafusion/physical-expr/src/expressions/binary.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1279,7 +1279,7 @@ mod tests {
12791279
use crate::expressions::{Column, Literal, col, lit, try_cast};
12801280
use datafusion_expr::lit as expr_lit;
12811281

1282-
use datafusion_common::plan_datafusion_err;
1282+
use datafusion_common::{assert_contains, plan_datafusion_err};
12831283
use datafusion_physical_expr_common::physical_expr::fmt_sql;
12841284

12851285
use crate::planner::logical2physical;
@@ -3331,6 +3331,33 @@ mod tests {
33313331
Ok(())
33323332
}
33333333

3334+
#[test]
3335+
fn regex_mismatched_array_types_error() -> Result<()> {
3336+
// The analyzer coerces both operands of a regex operator to a common
3337+
// string type, but an expression that bypasses it (e.g. constructed
3338+
// directly) must return an error instead of panicking
3339+
// (https://github.com/apache/datafusion/issues/22886)
3340+
let schema = Schema::new(vec![
3341+
Field::new("a", DataType::Utf8View, true),
3342+
Field::new("b", DataType::Utf8, true),
3343+
]);
3344+
let a = Arc::new(StringViewArray::from(vec!["user auth failed"])) as ArrayRef;
3345+
let b = Arc::new(StringArray::from(vec!["(auth|login)"])) as ArrayRef;
3346+
3347+
// construct the expression directly, without coercion
3348+
let expr = binary(
3349+
col("a", &schema)?,
3350+
Operator::RegexMatch,
3351+
col("b", &schema)?,
3352+
&schema,
3353+
)?;
3354+
let batch = RecordBatch::try_new(Arc::new(schema), vec![a, b])?;
3355+
let err = expr.evaluate(&batch).unwrap_err();
3356+
assert_contains!(err.to_string(), "failed to downcast array");
3357+
3358+
Ok(())
3359+
}
3360+
33343361
#[test]
33353362
fn or_with_nulls_op() -> Result<()> {
33363363
let schema = Schema::new(vec![

datafusion/physical-expr/src/expressions/binary/kernels.rs

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use arrow::compute::kernels::boolean::not;
2727
use arrow::compute::kernels::comparison::{regexp_is_match, regexp_is_match_scalar};
2828
use arrow::datatypes::DataType;
2929
use datafusion_common::{Result, ScalarValue};
30-
use datafusion_common::{internal_err, plan_err};
30+
use datafusion_common::{exec_err, internal_err, plan_err};
3131

3232
use std::sync::Arc;
3333

@@ -162,14 +162,27 @@ create_left_integral_dyn_scalar_kernel!(
162162
/// Invoke a compute kernel on a pair of binary data arrays with flags
163163
macro_rules! regexp_is_match_flag {
164164
($LEFT:expr, $RIGHT:expr, $ARRAYTYPE:ident, $NOT:expr, $FLAG:expr) => {{
165-
let ll = $LEFT
166-
.as_any()
167-
.downcast_ref::<$ARRAYTYPE>()
168-
.expect("failed to downcast array");
169-
let rr = $RIGHT
170-
.as_any()
171-
.downcast_ref::<$ARRAYTYPE>()
172-
.expect("failed to downcast array");
165+
// The analyzer coerces both operands to a common string type, but
166+
// expressions that bypass it may still reach here with mismatched
167+
// types, which must surface as an error rather than a panic.
168+
let ll = match $LEFT.as_any().downcast_ref::<$ARRAYTYPE>() {
169+
Some(ll) => ll,
170+
None => {
171+
return exec_err!(
172+
"failed to downcast array to {} for operation 'regex_match_dyn'",
173+
stringify!($ARRAYTYPE)
174+
);
175+
}
176+
};
177+
let rr = match $RIGHT.as_any().downcast_ref::<$ARRAYTYPE>() {
178+
Some(rr) => rr,
179+
None => {
180+
return exec_err!(
181+
"failed to downcast array to {} for operation 'regex_match_dyn'",
182+
stringify!($ARRAYTYPE)
183+
);
184+
}
185+
};
173186

174187
let flag = if $FLAG {
175188
Some($ARRAYTYPE::from(vec!["i"; ll.len()]))
@@ -210,10 +223,15 @@ pub(crate) fn regex_match_dyn(
210223
/// Invoke a compute kernel on a data array and a scalar value with flag
211224
macro_rules! regexp_is_match_flag_scalar {
212225
($LEFT:expr, $RIGHT:expr, $ARRAYTYPE:ident, $NOT:expr, $FLAG:expr) => {{
213-
let ll = $LEFT
214-
.as_any()
215-
.downcast_ref::<$ARRAYTYPE>()
216-
.expect("failed to downcast array");
226+
let ll = match $LEFT.as_any().downcast_ref::<$ARRAYTYPE>() {
227+
Some(ll) => ll,
228+
None => {
229+
return Some(exec_err!(
230+
"failed to downcast array to {} for operation 'regex_match_dyn_scalar'",
231+
stringify!($ARRAYTYPE)
232+
));
233+
}
234+
};
217235

218236
if let Some(Some(string_value)) = $RIGHT.try_as_str() {
219237
let flag = $FLAG.then_some("i");

0 commit comments

Comments
 (0)