Skip to content

Commit fa4ff20

Browse files
committed
Restore lazy evaluation of fallible CASE
Commit 0f4b8b1 introduced a new optimized evaluation mode for CASE expression with exactly one WHEN-THEN clause and ELSE clause being present. Apparently, the new mode did not take into account expensive or fallible expressions, as it unconditionally evaluated both branches. This is definitely incorrect for fallible expressions. For these, fallback to the original execution mode.
1 parent f32d86a commit fa4ff20

File tree

2 files changed

+29
-16
lines changed
  • datafusion

2 files changed

+29
-16
lines changed

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

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,10 @@ impl CaseExpr {
156156
&& else_expr.as_ref().unwrap().as_any().is::<Literal>()
157157
{
158158
EvalMethod::ScalarOrScalar
159-
} else if when_then_expr.len() == 1 && else_expr.is_some() {
159+
} else if when_then_expr.len() == 1
160+
&& is_cheap_and_infallible(&(when_then_expr[0].1))
161+
&& else_expr.as_ref().is_some_and(is_cheap_and_infallible)
162+
{
160163
EvalMethod::ExpressionOrExpression
161164
} else {
162165
EvalMethod::NoExpression
@@ -813,9 +816,18 @@ mod tests {
813816
}
814817

815818
fn case_test_batch1() -> Result<RecordBatch> {
816-
let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
819+
let schema = Schema::new(vec![
820+
Field::new("a", DataType::Int32, true),
821+
Field::new("b", DataType::Int32, true),
822+
Field::new("c", DataType::Int32, true),
823+
]);
817824
let a = Int32Array::from(vec![Some(1), Some(0), None, Some(5)]);
818-
let batch = RecordBatch::try_new(Arc::new(schema), vec![Arc::new(a)])?;
825+
let b = Int32Array::from(vec![Some(3), None, Some(14), Some(7)]);
826+
let c = Int32Array::from(vec![Some(0), Some(-3), Some(777), None]);
827+
let batch = RecordBatch::try_new(
828+
Arc::new(schema),
829+
vec![Arc::new(a), Arc::new(b), Arc::new(c)],
830+
)?;
819831
Ok(batch)
820832
}
821833

@@ -1306,18 +1318,8 @@ mod tests {
13061318
lit(2i32),
13071319
&batch.schema(),
13081320
)?;
1309-
let then = binary(
1310-
col("a", &schema)?,
1311-
Operator::Plus,
1312-
lit(1i32),
1313-
&batch.schema(),
1314-
)?;
1315-
let else_expr = binary(
1316-
col("a", &schema)?,
1317-
Operator::Minus,
1318-
lit(1i32),
1319-
&batch.schema(),
1320-
)?;
1321+
let then = col("b", &schema)?;
1322+
let else_expr = col("c", &schema)?;
13211323
let expr = CaseExpr::try_new(None, vec![(when, then)], Some(else_expr))?;
13221324
assert!(matches!(
13231325
expr.eval_method,
@@ -1329,7 +1331,7 @@ mod tests {
13291331
.expect("Failed to convert to array");
13301332
let result = as_int32_array(&result).expect("failed to downcast to Int32Array");
13311333

1332-
let expected = &Int32Array::from(vec![Some(2), Some(1), None, Some(4)]);
1334+
let expected = &Int32Array::from(vec![Some(3), None, Some(777), None]);
13331335

13341336
assert_eq!(expected, result);
13351337
Ok(())

datafusion/sqllogictest/test_files/case.slt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,18 @@ FROM t;
467467
----
468468
[{foo: blarg}]
469469

470+
query II
471+
SELECT v, CASE WHEN v != 0 THEN 10/v ELSE 42 END FROM (VALUES (0), (1), (2)) t(v)
472+
----
473+
0 42
474+
1 10
475+
2 5
470476

477+
query II
478+
SELECT v, CASE WHEN v < 0 THEN 10/0 ELSE 1 END FROM (VALUES (1), (2)) t(v)
479+
----
480+
1 1
481+
2 1
471482

472483
statement ok
473484
drop table t

0 commit comments

Comments
 (0)