Skip to content

Commit 3d52a60

Browse files
committed
fix: preserve first CASE condition evaluation
1 parent 49c02ec commit 3d52a60

2 files changed

Lines changed: 32 additions & 51 deletions

File tree

‎datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs‎

Lines changed: 27 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2438,22 +2438,16 @@ fn simplify_inlist_set_operation(
24382438

24392439
/// Conservatively checks the inputs whose evaluation can change when lowering
24402440
/// CASE to AND/OR. [`Expr`] has no general fallibility analysis: only columns and
2441-
/// literals are admitted from conditional branches, including later WHEN conditions.
2442-
/// The first WHEN already runs on every row, but must not be volatile because
2443-
/// the rewrite can evaluate it more than once.
2441+
/// literals are admitted for all WHEN conditions and branch outputs.
24442442
fn can_lower_case_to_boolean(
24452443
when_then_expr: &[(Box<Expr>, Box<Expr>)],
24462444
else_expr: Option<&Expr>,
24472445
) -> bool {
24482446
let is_leaf = |expr: &Expr| matches!(expr, Expr::Column(_) | Expr::Literal(..));
2449-
when_then_expr.iter().enumerate().all(|(i, (when, then))| {
2450-
is_leaf(then)
2451-
&& if i == 0 {
2452-
!when.is_volatile()
2453-
} else {
2454-
is_leaf(when)
2455-
}
2456-
}) && else_expr.is_none_or(is_leaf)
2447+
when_then_expr
2448+
.iter()
2449+
.all(|(when, then)| is_leaf(when) && is_leaf(then))
2450+
&& else_expr.is_none_or(is_leaf)
24572451
}
24582452

24592453
/// Returns expression testing a boolean `expr` for being exactly `true` (not `false` or NULL).
@@ -4289,6 +4283,11 @@ mod tests {
42894283
vec![],
42904284
));
42914285
for expr in [
4286+
Expr::Case(Case::new(
4287+
None,
4288+
vec![(Box::new(fallible.clone()), Box::new(lit(false)))],
4289+
Some(Box::new(lit(false))),
4290+
)),
42924291
Expr::Case(Case::new(
42934292
None,
42944293
vec![(Box::new(col("c2")), Box::new(fallible.clone()))],
@@ -4358,19 +4357,20 @@ mod tests {
43584357

43594358
// CASE WHEN ISNULL(c2) THEN true ELSE c2
43604359
// -->
4361-
// ISNULL(c2) OR c2
4362-
//
4363-
// Need to call simplify 2x due to
4364-
// https://github.com/apache/datafusion/issues/1160
4360+
// Preserve CASE because the WHEN expression is outside the conservative
4361+
// column/literal subset.
4362+
let expected = Expr::Case(Case::new(
4363+
None,
4364+
vec![(Box::new(col("c2").is_null()), Box::new(lit(true)))],
4365+
Some(Box::new(col("c2"))),
4366+
));
43654367
assert_eq!(
43664368
simplify(simplify(Expr::Case(Case::new(
43674369
None,
43684370
vec![(Box::new(col("c2").is_null()), Box::new(lit(true)),)],
43694371
Some(Box::new(col("c2"))),
43704372
)))),
4371-
col("c2")
4372-
.is_null()
4373-
.or(col("c2").is_not_null().and(col("c2")))
4373+
expected
43744374
);
43754375

43764376
// CASE WHEN c1 then true WHEN c2 then false ELSE true
@@ -4411,29 +4411,21 @@ mod tests {
44114411
col("c1_non_null").or(col("c1_non_null").not().and(col("c2_non_null").not()))
44124412
);
44134413

4414-
// CASE WHEN c > 0 THEN true END AS c1
4415-
assert_eq!(
4416-
simplify(simplify(Expr::Case(Case::new(
4414+
// Preserve CASE with a non-leaf WHEN condition, with or without ELSE.
4415+
for expr in [
4416+
Expr::Case(Case::new(
44174417
None,
44184418
vec![(Box::new(col("c3").gt(lit(0_i64))), Box::new(lit(true)))],
44194419
None,
4420-
)))),
4421-
not_distinct_from(col("c3").gt(lit(0_i64)), lit(true)).or(distinct_from(
4422-
col("c3").gt(lit(0_i64)),
4423-
lit(true)
4424-
)
4425-
.and(lit_bool_null()))
4426-
);
4427-
4428-
// CASE WHEN c > 0 THEN true ELSE false END AS c1
4429-
assert_eq!(
4430-
simplify(simplify(Expr::Case(Case::new(
4420+
)),
4421+
Expr::Case(Case::new(
44314422
None,
44324423
vec![(Box::new(col("c3").gt(lit(0_i64))), Box::new(lit(true)))],
44334424
Some(Box::new(lit(false))),
4434-
)))),
4435-
not_distinct_from(col("c3").gt(lit(0_i64)), lit(true))
4436-
);
4425+
)),
4426+
] {
4427+
assert_eq!(simplify(simplify(expr.clone())), expr);
4428+
}
44374429
}
44384430

44394431
#[test]
@@ -4630,22 +4622,6 @@ mod tests {
46304622
assert_eq!(simplify(expr.clone()), expr);
46314623
}
46324624

4633-
fn distinct_from(left: impl Into<Expr>, right: impl Into<Expr>) -> Expr {
4634-
Expr::BinaryExpr(BinaryExpr {
4635-
left: Box::new(left.into()),
4636-
op: Operator::IsDistinctFrom,
4637-
right: Box::new(right.into()),
4638-
})
4639-
}
4640-
4641-
fn not_distinct_from(left: impl Into<Expr>, right: impl Into<Expr>) -> Expr {
4642-
Expr::BinaryExpr(BinaryExpr {
4643-
left: Box::new(left.into()),
4644-
op: Operator::IsNotDistinctFrom,
4645-
right: Box::new(right.into()),
4646-
})
4647-
}
4648-
46494625
#[test]
46504626
fn simplify_expr_bool_or() {
46514627
// col || true is always true

‎datafusion/sqllogictest/test_files/case.slt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,5 +962,10 @@ query error Cannot cast string 'abc' to value of Int32 type
962962
SELECT CASE WHEN s ~ '^[a-z]+$' THEN CAST(s AS INT) > 0 ELSE false END
963963
FROM guarded_cast;
964964

965+
# The first WHEN must be evaluated even when both branch outputs are identical.
966+
query error Cannot cast string 'abc' to value of Int32 type
967+
SELECT CASE WHEN CAST(s AS INT) > 0 THEN false ELSE false END
968+
FROM (VALUES ('abc')) t(s);
969+
965970
statement ok
966971
DROP VIEW guarded_cast;

0 commit comments

Comments
 (0)