Skip to content

Commit dfdf367

Browse files
u70b3claude
andcommitted
fix: return error instead of panic on downcast failure in regex kernels
Defense in depth for #22886: `regexp_is_match_flag!` and `regexp_is_match_flag_scalar!` used `.expect("failed to downcast array")` on both operands. Any expression path that bypasses the analyzer's type coercion (e.g. a hand-built plan going straight to physical planning) would panic instead of returning a DataFusion error. Return `exec_err!` instead; the fast paths are unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 8ae08fc commit dfdf367

2 files changed

Lines changed: 59 additions & 14 deletions

File tree

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)