This is an automated email from the ASF dual-hosted git repository.
morrySnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 08fd5f81f52 [fix](aggregate) Restrict sum literal reassociation to
integers (#67801)
08fd5f81f52 is described below
commit 08fd5f81f529a1b98f0d573495319eff4ef49dff
Author: morrySnow <[email protected]>
AuthorDate: Mon Sep 14 10:55:39 2026 +0800
[fix](aggregate) Restrict sum literal reassociation to integers (#67801)
## Problem
When a query contains multiple aggregates such as SUM(x + 1.0) and SUM(x
+ 2.0), the optimizer reassociates them into SUM(x) plus COUNT(x) times
each literal. For floating-point inputs this changes IEEE-754 rounding
from per-row addition to post-aggregation addition and can return a
different result.
## Root cause
SumLiteralRewrite accepted both integer and floating-point literals, but
did not require the analyzed input expression and arithmetic result to
be integer types. The transformation is not semantics-preserving for
floating-point arithmetic.
## Reproduction
With DOUBLE values 1e16 and -1e16, the two original aggregates produce 0
and 4. Before this change, the rewritten plan produced 2 and 4 and
exposed COUNT in the aggregate plan.
## Fix
Restrict the reassociation to expressions whose analyzed child,
non-literal operand, and literal operand are all integer-like.
Floating-point and decimal expressions remain unchanged. Existing
widening integer-cast handling is preserved. Integer overflow behavior
is intentionally unchanged.
## Tests
- Added unit coverage for integer, float, double, decimal, and mixed
operand types.
- Added a regression case for the floating-point rounding reproduction
and verified that its plan is not rewritten.
- Verified that integer aggregates still use the optimization.
- Ran the focused FE unit test, the full FE build with checkstyle, and
the sumRewrite regression suite.
---
.../nereids/rules/rewrite/SumLiteralRewrite.java | 7 ++-
.../rules/rewrite/SumLiteralRewriteTest.java | 53 +++++++++++++++++++++-
.../suites/nereids_rules_p0/sumRewrite.groovy | 28 ++++++++++++
3 files changed, 85 insertions(+), 3 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java
index cf983c03133..8d91531a04b 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java
@@ -201,8 +201,11 @@ public class SumLiteralRewrite extends
OneRewriteRuleFactory {
// right now, only support expr +/- literal
return null;
}
- if (!(right.getDataType().isIntegerLikeType() ||
right.getDataType().isFloatLikeType())) {
- // only support integer or float types
+ if (!child.getDataType().isIntegerLikeType()
+ || !left.getDataType().isIntegerLikeType()
+ || !right.getDataType().isIntegerLikeType()) {
+ // Reassociation changes per-row rounding for floating-point types.
+ // Only rewrite arithmetic whose analyzed operands and result are
integers.
return null;
}
// Strip redundant widening integer cast introduced by type coercion.
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewriteTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewriteTest.java
index 5b918c62a59..1a8d2470e8e 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewriteTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewriteTest.java
@@ -21,12 +21,18 @@ import org.apache.doris.nereids.trees.expressions.Add;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Cast;
import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.Subtract;
import org.apache.doris.nereids.trees.expressions.functions.agg.Sum;
+import org.apache.doris.nereids.trees.expressions.literal.DecimalV3Literal;
+import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.FloatLiteral;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
import org.apache.doris.nereids.types.BigIntType;
+import org.apache.doris.nereids.types.DoubleType;
+import org.apache.doris.nereids.types.FloatType;
import org.apache.doris.nereids.util.MemoPatternMatchSupported;
import org.apache.doris.nereids.util.MemoTestUtils;
import org.apache.doris.nereids.util.PlanChecker;
@@ -35,6 +41,8 @@ import org.apache.doris.nereids.util.PlanConstructor;
import com.google.common.collect.ImmutableList;
import org.junit.jupiter.api.Test;
+import java.math.BigDecimal;
+
class SumLiteralRewriteTest implements MemoPatternMatchSupported {
private final LogicalOlapScan scan1 =
PlanConstructor.newLogicalOlapScan(0, "t1", 0);
@@ -118,6 +126,8 @@ class SumLiteralRewriteTest implements
MemoPatternMatchSupported {
@Test
void testSumOnce() {
Slot slot1 = scan1.getOutput().get(0);
+ // Create the independent slot before PlanChecker initializes a new
statement ID scope.
+ Slot slot2 = new Alias(scan1.getOutput().get(0)).toSlot();
Alias add1 = new Alias(new Sum(false, true, new Add(slot1,
Literal.of(1))));
LogicalAggregate<?> agg = new LogicalAggregate<>(
ImmutableList.of(scan1.getOutput().get(0)),
ImmutableList.of(add1), scan1);
@@ -126,7 +136,6 @@ class SumLiteralRewriteTest implements
MemoPatternMatchSupported {
.printlnTree()
.matches(logicalAggregate().when(p -> p.getOutputs().size() ==
1));
- Slot slot2 = new Alias(scan1.getOutput().get(0)).toSlot();
Alias add2 = new Alias(new Sum(false, true, new Add(slot2,
Literal.of(2))));
agg = new LogicalAggregate<>(
ImmutableList.of(scan1.getOutput().get(0)),
ImmutableList.of(add1, add2), scan1);
@@ -146,6 +155,48 @@ class SumLiteralRewriteTest implements
MemoPatternMatchSupported {
}
+ @Test
+ void testOnlyRewriteIntegerArithmetic() {
+ Slot integerSlot = scan1.getOutput().get(0);
+ LogicalAggregate<?> integerAgg = new LogicalAggregate<>(
+ ImmutableList.of(),
+ ImmutableList.of(
+ new Alias(new Sum(new Add(integerSlot,
Literal.of(1)))),
+ new Alias(new Sum(new Add(integerSlot,
Literal.of(2))))),
+ scan1);
+ PlanChecker.from(MemoTestUtils.createConnectContext(), integerAgg)
+ .applyTopDown(ImmutableList.of(new
SumLiteralRewrite().build()))
+ .matchesFromRoot(logicalProject(logicalAggregate()));
+
+ assertSumLiteralNotRewritten(
+ new SlotReference("float_slot", FloatType.INSTANCE),
+ new FloatLiteral(1.0F), new FloatLiteral(2.0F));
+ assertSumLiteralNotRewritten(
+ new SlotReference("double_slot", DoubleType.INSTANCE),
+ new DoubleLiteral(1.0), new DoubleLiteral(2.0));
+
+ DecimalV3Literal decimalOne = new DecimalV3Literal(new
BigDecimal("1.0"));
+ DecimalV3Literal decimalTwo = new DecimalV3Literal(new
BigDecimal("2.0"));
+ assertSumLiteralNotRewritten(
+ new SlotReference("decimal_slot", decimalOne.getDataType()),
decimalOne, decimalTwo);
+
+ // Also reject a non-integer input if an uncoerced integer literal
reaches this rule.
+ assertSumLiteralNotRewritten(
+ new SlotReference("mixed_slot", DoubleType.INSTANCE),
Literal.of(1), Literal.of(2));
+ }
+
+ private void assertSumLiteralNotRewritten(Slot slot, Literal first,
Literal second) {
+ LogicalAggregate<?> agg = new LogicalAggregate<>(
+ ImmutableList.of(),
+ ImmutableList.of(
+ new Alias(new Sum(new Add(slot, first))),
+ new Alias(new Sum(new Add(slot, second)))),
+ scan1);
+ PlanChecker.from(MemoTestUtils.createConnectContext(), agg)
+ .applyTopDown(ImmutableList.of(new
SumLiteralRewrite().build()))
+ .matchesFromRoot(logicalAggregate());
+ }
+
@Test
void testStripWideningIntegerCast() {
Slot slot1 = scan1.getOutput().get(0);
diff --git a/regression-test/suites/nereids_rules_p0/sumRewrite.groovy
b/regression-test/suites/nereids_rules_p0/sumRewrite.groovy
index c9d492bb06b..e3714d546a5 100644
--- a/regression-test/suites/nereids_rules_p0/sumRewrite.groovy
+++ b/regression-test/suites/nereids_rules_p0/sumRewrite.groovy
@@ -124,4 +124,32 @@ INSERT INTO sr (id, not_null_id, f_id, d_id) VALUES
// explainAndOrderResult 'decimal_sum_sub_const_precision_3', """ select
not_null_id, sum(d_id - 2) from sr group by not_null_id """
// explainAndOrderResult 'decimal_sum_sub_const_precision_4', """ select
not_null_id, sum(d_id - 2.223) from sr group by not_null_id """
+
+ sql "DROP TABLE IF EXISTS sr_double_rounding"
+ sql """
+ CREATE TABLE sr_double_rounding (
+ id INT NOT NULL,
+ x DOUBLE NULL
+ ) ENGINE = OLAP
+ DUPLICATE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES ("replication_num" = "1")
+ """
+ sql "INSERT INTO sr_double_rounding VALUES (1, 1e16), (2, -1e16)"
+
+ def doubleResults = sql """
+ SELECT CAST(SUM(x + 1.0) AS STRING), CAST(SUM(x + 2.0) AS STRING)
+ FROM sr_double_rounding
+ """
+ assertEquals(1, doubleResults.size())
+ assertEquals("0", doubleResults[0][0])
+ assertEquals("4", doubleResults[0][1])
+
+ explain {
+ sql """
+ SELECT SUM(x + 1.0), SUM(x + 2.0)
+ FROM sr_double_rounding
+ """
+ notContains "count("
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
