diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyArithmeticRule.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyArithmeticRule.java index 3076e36ec52e11..312721ba16bf14 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyArithmeticRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyArithmeticRule.java @@ -143,6 +143,14 @@ private static List flatten(Expression expr, boolean isAddOrSub) { // flag: true for '+' or '*', false for '-' or '/' // isAddOrSub: true for extract only "+" or "-" sub expressions, false for extract only "*" or "/" sub expressions private static void doFlatten(boolean flag, Expression expr, boolean isAddOrSub, List result) { + if (!isAddOrSub && !flag) { + // A complete denominator is an evaluation boundary. Flattening its multiply/divide + // children into the enclosing expression can invert them or move them to the numerator, + // changing division-by-zero, null, overflow, and floating-point behavior. Keep the + // subtree atomic here; process() will still simplify it recursively within its boundary. + result.add(Operand.of(false, expr)); + return; + } BinaryArithmetic arithmetic = null; Predicate isPositiveArithmetic = isAddOrSub ? TypeUtils::isAdd : TypeUtils::isMultiply; diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyArithmeticRuleTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyArithmeticRuleTest.java index cb09906214bbc9..0d4136069db6c0 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyArithmeticRuleTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/SimplifyArithmeticRuleTest.java @@ -56,6 +56,34 @@ void testSimplifyArithmetic() { "(((((IA / ((IB + (IC * (-56 / 50))) + 36)) * ID) / (IE + 50)) * (((40 / 20) * 5) * 6)) + (1 + 200))"); } + @Test + void testPreserveDivisionDenominatorBoundaries() { + executor = new ExpressionRuleExecutor(ImmutableList.of( + bottomUp(SimplifyArithmeticRule.INSTANCE, FoldConstantRule.INSTANCE) + )); + + Assertions.assertAll( + // A nested denominator is an evaluation boundary: its operands cannot be + // inverted or moved into the numerator of the enclosing division. + () -> assertRewriteAfterSimplify("IA / (IB / IC)", "IA / (IB / IC)"), + () -> assertRewriteAfterSimplify("IA / (IB * IC)", "IA / (IB * IC)"), + + // Left-associated division does not introduce a nested denominator boundary. + () -> assertRewriteAfterSimplify("(IA / IB) / IC", "(IA / IB) / IC"), + + // Preserve zero and null behavior for nullable integer operands. + () -> assertRewriteAfterSimplify("1 / (1 / IA)", "1 / (1 / IA)"), + () -> assertRewriteAfterSimplify("IA / (IB / 0)", "IA / (IB / 0)"), + () -> assertRewriteAfterSimplify("IA / (IB * 0)", "IA / (IB * 0)"), + + // Floating-point arithmetic has the same evaluation-order boundary. + () -> assertRewriteAfterSimplify("DA / (DB / DC)", "DA / (DB / DC)"), + () -> assertRewriteAfterSimplify("DA / (DB * DC)", "DA / (DB * DC)"), + + // Decimal arithmetic is intentionally unsupported by this rule and remains unchanged. + () -> assertRewriteAfterSimplify("MA / (MB / MC)", "MA / (MB / MC)")); + } + @Test void testSimplifyArithmeticRuleOnly() { executor = new ExpressionRuleExecutor(ImmutableList.of( @@ -71,10 +99,14 @@ void testSimplifyArithmeticRuleOnly() { assertRewriteAfterSimplify("IA - 2 - ((-IB - 1) - (3 + (IC + 4)))", "(((IA + IB) + IC) - ((((2 + 0) - 1) - 3) - 4))"); // multiply and divide - assertRewriteAfterSimplify("2 / IA / ((1 / IB) / (3 * IC))", "(((((2 / 1) * 3) / IA) * IB) * IC)"); - assertRewriteAfterSimplify("IA / 2 / ((IB * 1) / (3 / (IC / 4)))", "(((IA / IB) / IC) / (((2 * 1) / 3) / 4))"); - assertRewriteAfterSimplify("IA / 2 / ((IB / 1) / (3 / (IC * 4)))", "(((IA / IB) / IC) / (((2 / 1) / 3) * 4))"); - assertRewriteAfterSimplify("IA / 2 / ((IB / 1) / (3 * (IC * 4)))", "(((IA / IB) * IC) / (((2 / 1) / 3) / 4))"); + assertRewriteAfterSimplify("2 / IA / ((1 / IB) / (3 * IC))", + "((2 / IA) / ((1 / IB) / (IC * 3)))"); + assertRewriteAfterSimplify("IA / 2 / ((IB * 1) / (3 / (IC / 4)))", + "((IA / ((IB / (3 / (IC / 4))) * 1)) / 2)"); + assertRewriteAfterSimplify("IA / 2 / ((IB / 1) / (3 / (IC * 4)))", + "((IA / ((IB / (3 / (IC * 4))) / 1)) / 2)"); + assertRewriteAfterSimplify("IA / 2 / ((IB / 1) / (3 * (IC * 4)))", + "((IA / ((IB / (IC * (3 * 4))) / 1)) / 2)"); // hybrid // root is subtract @@ -88,7 +120,8 @@ void testSimplifyArithmeticRuleOnly() { assertRewriteAfterSimplify("-IA / 2 * ((-IB - 1) * (3 / (IC + 4)))", "((((0 - IA) * ((0 - 1) - IB)) / (IC + 4)) / (2 / 3))"); // root is divide assertRewriteAfterSimplify("(-IA / 2) / ((-IB - 1) - (3 + (IC + 4)))", "(((0 - IA) / (((((0 - 1) - 3) - 4) - IB) - IC)) / 2)"); - assertRewriteAfterSimplify("(-IA / 2) / ((-IB - 1) / (3 + (IC * 4)))", "((((0 - IA) / ((0 - 1) - IB)) * ((IC * 4) + 3)) / 2)"); + assertRewriteAfterSimplify("(-IA / 2) / ((-IB - 1) / (3 + (IC * 4)))", + "(((0 - IA) / (((0 - 1) - IB) / ((IC * 4) + 3))) / 2)"); // unsupported decimal assertRewriteAfterSimplify("-2 - MA - ((1 - IB) - (3 + IC))", "((-2 - MA) - ((1 - IB) - (3 + IC)))"); diff --git a/regression-test/data/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.out b/regression-test/data/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.out index e7c6a5a1dd5cd0..fa813f476a962b 100644 --- a/regression-test/data/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.out +++ b/regression-test/data/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.out @@ -1,3 +1,16 @@ -- This file is automatically generated. You should know what you did if you want to edit this -- !return_type_after_projection_should_be_bigint -- +-- !preserve_division_denominator_in_projection -- +0 \N +1 1 +2 2 + +-- !preserve_division_denominator_in_filter -- +0 + +-- !preserve_division_denominator_rule_disabled -- +0 \N +1 1 +2 2 + diff --git a/regression-test/suites/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.groovy b/regression-test/suites/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.groovy index bc1fc20ee8fe9b..a4f0e3cf234d39 100644 --- a/regression-test/suites/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.groovy +++ b/regression-test/suites/nereids_rules_p0/expression/simplify_arithmetic/test_simplify_arithmetic.groovy @@ -36,4 +36,36 @@ suite("test_simplify_arithmetic") { qt_return_type_after_projection_should_be_bigint """ select -3 - (7 + id) as c1 from test_simplify_arithmetic group by c1 """ + + // A nested denominator is an evaluation boundary. In particular, rewriting this to + // number * 1 would change the number = 0 result from NULL to 0. + explain { + sql """ + select number, 1 / (1 / number) as result + from numbers("number" = "3") + """ + verbose true + contains """(1 / (1 / CAST(""" + } + + qt_preserve_division_denominator_in_projection """ + select number, 1 / (1 / number) as result + from numbers("number" = "3") + order by number + """ + + qt_preserve_division_denominator_in_filter """ + select number + from numbers("number" = "3") + where 1 / (1 / number) is null + order by number + """ + + sql "set disable_nereids_expression_rules='SIMPLIFY_ARITHMETIC'" + qt_preserve_division_denominator_rule_disabled """ + select number, 1 / (1 / number) as result + from numbers("number" = "3") + order by number + """ + sql "set disable_nereids_expression_rules=''" }