Skip to content

Commit

Permalink
Fix zero type in expr % 1 simplification (#12913)
Browse files Browse the repository at this point in the history
This address a bug that previously always replace % 1 expression with a
0 of type i32. This lead to panics/crashes in a lot of places since we
expect the type to not change as part of this simplification rule. This
patch fixes it by replacing it with a 0 of correct type.

This was discovered in #12814
  • Loading branch information
eejbyfeldt authored Oct 16, 2024
1 parent f8add9b commit 0e08e06
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,9 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for Simplifier<'a, S> {
&& !info.get_data_type(&left)?.is_floating()
&& is_one(&right) =>
{
Transformed::yes(lit(0))
Transformed::yes(Expr::Literal(ScalarValue::new_zero(
&info.get_data_type(&left)?,
)?))
}

//
Expand Down Expand Up @@ -2163,11 +2165,11 @@ mod tests {

#[test]
fn test_simplify_modulo_by_one_non_null() {
let expr = col("c2_non_null") % lit(1);
let expected = lit(0);
let expr = col("c3_non_null") % lit(1);
let expected = lit(0_i64);
assert_eq!(simplify(expr), expected);
let expr =
col("c2_non_null") % lit(ScalarValue::Decimal128(Some(10000000000), 31, 10));
col("c3_non_null") % lit(ScalarValue::Decimal128(Some(10000000000), 31, 10));
assert_eq!(simplify(expr), expected);
}

Expand Down

0 comments on commit 0e08e06

Please sign in to comment.