This is an automated email from the ASF dual-hosted git repository.
englefly 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 c45d1a60670 [fix](eager-agg) Handle duplicate aggregate functions
pushed through projects (#66531)
c45d1a60670 is described below
commit c45d1a6067067ee6066e92275a2013e858e917d1
Author: feiniaofeiafei <[email protected]>
AuthorDate: Wed Aug 12 11:04:19 2026 +0800
[fix](eager-agg) Handle duplicate aggregate functions pushed through
projects (#66531)
### What problem does this PR solve?
Related PR: #63690
Problem Summary:
Eager aggregation pushdown may fail when different aggregate functions
become
the same expression after passing through a Project. report error:
```text
2026-08-06 04:26:54,285 INFO (mysql-nio-pool-14|325)
[PushDownAggregation.visitLogicalAggregate():280] PushDownAggregation failed:
Cannot invoke
"org.apache.doris.nereids.trees.expressions.NamedExpression.toSlot()" because
"namedExpression" is null
at
org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalProject(EagerAggRewriter.java:718)
at
org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalProject(EagerAggRewriter.java:90)
at
org.apache.doris.nereids.trees.plans.logical.LogicalProject.accept(LogicalProject.java:160)
at
org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalUnion(EagerAggRewriter.java:582)
at
org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriter.visitLogicalUnion(EagerAggRewriter.java:90)
at
org.apache.doris.nereids.trees.plans.logical.LogicalUnion.accept(LogicalUnion.java:155)
```
For example:
```text
Aggregate: SUM(x)#4, SUM(y)#5
Union All
Project: 0 AS x, 0 AS y
Join
```
After pushing the aggregates through the Project, both functions
become`SUM(0)`:
```text
functions: [SUM(0), SUM(0)]
aliasMap: SUM(0) -> #5
```
Because `aliasMap` uses expression equality, only one entry is retained.
The Project still tries to read both `#4` and `#5` from
`BilateralState`,
causing a null lookup.
This PR deduplicates the child aggregate and records the ExprId mapping:
```text
child aggregate: SUM(0) -> #8
ExprId mapping: #4 -> #8, #5 -> #8
```
The Project then restores both required outputs:
```text
slot#8 AS slot#4
slot#8 AS slot#5
```
When no aggregate functions are merged, the original ExprIds are reused
to
avoid unnecessary aliases.
The same fix also covers cases such as:
```text
Project: a#1 AS x, a#1 AS y
```
where `SUM(x)` and `SUM(y)` both become `SUM(a#1)` after pushdown.
### Release note
None
### Check List (For Author)
- Test
- [x] Regression test
- `query_p0/eager_agg/bilateral_eager_agg`
- Covers two aggregate functions that become the same function after
Project pushdown.
- [ ] Unit Test
- [ ] Manual test
- [ ] No need to test or manual test.
- Behavior changed:
- [x] Yes.
- Prevents eager aggregation pushdown from failing when multiple
aggregate functions become identical after Project rewriting.
- `eager_aggregation_mode=1` can force eligible pushdown after a UNION.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
---
.../rewrite/eageraggregation/EagerAggRewriter.java | 78 +++++++++++++---------
.../query_p0/eager_agg/bilateral_eager_agg.out | 4 ++
.../data/query_p0/eager_agg/eager_agg.out | 8 ++-
.../query_p0/eager_agg/bilateral_eager_agg.groovy | 74 ++++++++++++++++++++
4 files changed, 131 insertions(+), 33 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriter.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriter.java
index a698459125c..e0219de031a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriter.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriter.java
@@ -60,11 +60,13 @@ import org.apache.doris.qe.SessionVariable;
import org.apache.doris.statistics.ColumnStatistic;
import org.apache.doris.statistics.Statistics;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.IdentityHashMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -134,8 +136,8 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
// construct left and right aggFuncs and aliasMap
List<AggregateFunction> leftFuncs = new ArrayList<>();
List<AggregateFunction> rightFuncs = new ArrayList<>();
- Map<AggregateFunction, Alias> leftAliasMap = new IdentityHashMap<>();
- Map<AggregateFunction, Alias> rightAliasMap = new IdentityHashMap<>();
+ Map<AggregateFunction, Alias> leftAliasMap = new HashMap<>();
+ Map<AggregateFunction, Alias> rightAliasMap = new HashMap<>();
for (AggregateFunction f : context.getAggFunctions()) {
Set<Slot> inputs = f.getInputSlots();
Alias a = context.getAliasMap().get(f);
@@ -197,7 +199,7 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
}
private boolean isPassThroughHeavyJoin(Plan joinChild, PushDownAggContext
context) {
- if (context.isPassThroughHeavyJoin() ||
SessionVariable.getEagerAggregationMode() > 0) {
+ if (context.isPassThroughHeavyJoin()) {
return true;
} else {
Statistics stats = joinChild.getStats();
@@ -405,7 +407,7 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
private PushDownAggContext createContextFromProject(
LogicalProject<? extends Plan> project,
- PushDownAggContext context) {
+ PushDownAggContext context, Map<ExprId, ExprId>
projectToChildExprIdMap) {
/*
* context: sum(a) groupBy(y+z as x, l)
* proj: b+c as a, u+v as y, m+n as l
@@ -419,35 +421,40 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
.stream().map(slot -> (SlotReference)
slot).collect(Collectors.toList()));
}
- List<AggregateFunction> aggFunctions = new ArrayList<>();
- Map<AggregateFunction, Alias> aliasMap = new IdentityHashMap<>();
+ Set<AggregateFunction> aggFunctions = new LinkedHashSet<>();
+ Map<AggregateFunction, Alias> aliasMap = new HashMap<>();
+ boolean newContainsNullToNonNull = context.containsNullToNonNull;
+
for (AggregateFunction aggFunc : context.getAggFunctions()) {
AggregateFunction newAggFunc = (AggregateFunction)
project.pushDownExpressionPastProject(aggFunc);
Alias alias = context.getAliasMap().get(aggFunc);
- aliasMap.put(newAggFunc, (Alias) alias.withChildren(newAggFunc));
+ Alias aliasForChild;
+ if (aliasMap.containsKey(newAggFunc)) {
+ aliasForChild = aliasMap.get(newAggFunc);
+ } else {
+ aliasForChild = (Alias) alias.withChildren(newAggFunc);
+ aliasMap.put(newAggFunc, aliasForChild);
+ }
+ projectToChildExprIdMap.put(alias.getExprId(),
aliasForChild.getExprId());
aggFunctions.add(newAggFunc);
- }
- // After pushing expressions past the project, the agg functions may
now
- // contain NullToNonNull expressions that were hidden behind slot
references before.
- // e.g. count(#slot) where #slot = coalesce(a, 0) in the project.
- // We must re-check and update containsNullToNonNull accordingly.
- boolean newContainsNullToNonNull = context.containsNullToNonNull;
- if (!newContainsNullToNonNull) {
- for (AggregateFunction aggFunc : aggFunctions) {
- if (aggFunc.children().stream().anyMatch(
- arg -> arg.anyMatch(e ->
-
NullToNonNullFunction.canConvertNullToNonNull((Expression) e)))) {
- newContainsNullToNonNull = true;
- break;
- }
+
+ // After pushing expressions past the project, the agg functions
may now
+ // contain NullToNonNull expressions that were hidden behind slot
references before.
+ // e.g. count(#slot) where #slot = coalesce(a, 0) in the project.
+ // We must re-check and update containsNullToNonNull accordingly.
+ if (!newContainsNullToNonNull
+ && newAggFunc.children().stream().anyMatch(
+ arg -> arg.anyMatch(e ->
+
NullToNonNullFunction.canConvertNullToNonNull((Expression) e)))) {
+ newContainsNullToNonNull = true;
}
}
- PushDownAggContext newContext = new PushDownAggContext(aggFunctions,
groupKeys, aliasMap,
+
+ return new PushDownAggContext(ImmutableList.copyOf(aggFunctions),
groupKeys, aliasMap,
context.getCascadesContext(), context.isPassThroughHeavyJoin(),
context.hasDecomposedAggIf, newContainsNullToNonNull,
context.getBilateralState(), context.needOutputCount(),
context.isPassThroughJoinOrUnion(),
context.isSmallBroadcastBottomJoin());
- return newContext;
}
private boolean canPushThroughProject(LogicalProject<? extends Plan>
project, PushDownAggContext context) {
@@ -557,7 +564,7 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
Plan child = union.children().get(idx);
final int childIdx = idx;
List<AggregateFunction> aggFunctionsForChild = new ArrayList<>();
- IdentityHashMap<AggregateFunction, Alias> aliasMapForChild = new
IdentityHashMap<>();
+ Map<AggregateFunction, Alias> aliasMapForChild = new HashMap<>();
for (AggregateFunction func : context.getAggFunctions()) {
AggregateFunction newFunc = (AggregateFunction)
union.pushDownExpressionPastSetOperator(func, childIdx);
aggFunctionsForChild.add(newFunc);
@@ -686,7 +693,8 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
if (!canPushThroughProject(project, context)) {
return genAggregate(project, context);
}
- PushDownAggContext newContext = createContextFromProject(project,
context);
+ Map<ExprId, ExprId> projectToChildExprIdMap = new HashMap<>();
+ PushDownAggContext newContext = createContextFromProject(project,
context, projectToChildExprIdMap);
if (newContext.aggFuncAndGroupKeyAllEmpty()) {
return project;
}
@@ -714,9 +722,19 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
BilateralState state = context.getBilateralState();
for (AggregateFunction aggFunc : context.getAggFunctions()) {
Alias alias = context.getAliasMap().get(aggFunc);
- NamedExpression namedExpression =
state.getPushedAggFuncSlot(alias.getExprId());
- newProjections.add(namedExpression.toSlot());
+ ExprId childExprId =
projectToChildExprIdMap.get(alias.getExprId());
+ NamedExpression namedExpression =
state.getPushedAggFuncSlot(childExprId);
+ NamedExpression output;
+ if (namedExpression.getExprId().equals(alias.getExprId())) {
+ output = namedExpression.toSlot();
+ } else {
+ output = (Alias)
alias.withChildren(namedExpression.toSlot());
+ state.registerAggFuncOutput(alias.getExprId(),
output.toSlot(),
+ state.isAggFuncActuallyPushed(childExprId));
+ }
+ newProjections.add(output);
}
+
for (SlotReference slot : context.getGroupKeys()) {
boolean valid = false;
for (NamedExpression ne : project.getProjects()) {
@@ -1304,9 +1322,7 @@ public class EagerAggRewriter extends
DefaultPlanRewriter<PushDownAggContext> {
}
if (mode > 0) {
- // when mode=1, any join is regarded as big join in order to
- // push down aggregation through at least one join
- return context.isPassThroughHeavyJoin();
+ return true;
}
if (!context.isPassThroughHeavyJoin() && !context.hasDecomposedAggIf) {
diff --git a/regression-test/data/query_p0/eager_agg/bilateral_eager_agg.out
b/regression-test/data/query_p0/eager_agg/bilateral_eager_agg.out
index a386ad329a1..1ff0e1a9dc8 100644
--- a/regression-test/data/query_p0/eager_agg/bilateral_eager_agg.out
+++ b/regression-test/data/query_p0/eager_agg/bilateral_eager_agg.out
@@ -335,3 +335,7 @@
2000-06-03 true
2020-01-01 false
+-- !union_2_same_agg_func --
+1 10 10
+2 20 20
+
diff --git a/regression-test/data/query_p0/eager_agg/eager_agg.out
b/regression-test/data/query_p0/eager_agg/eager_agg.out
index c0ed2babc99..b044573b029 100644
--- a/regression-test/data/query_p0/eager_agg/eager_agg.out
+++ b/regression-test/data/query_p0/eager_agg/eager_agg.out
@@ -307,8 +307,12 @@ PhysicalResultSink
------PhysicalUnion
--------hashJoin[INNER_JOIN] hashCondition=((dt.d_date_sk =
ss.ss_sold_date_sk)) otherCondition=()
----------PhysicalOlapScan[store_sales(ss)]
-----------PhysicalOlapScan[date_dim(dt)]
---------PhysicalOlapScan[date_dim]
+----------hashAgg[GLOBAL]
+------------hashAgg[LOCAL]
+--------------PhysicalOlapScan[date_dim(dt)]
+--------hashAgg[GLOBAL]
+----------hashAgg[LOCAL]
+------------PhysicalOlapScan[date_dim]
Hint log:
Used:
diff --git
a/regression-test/suites/query_p0/eager_agg/bilateral_eager_agg.groovy
b/regression-test/suites/query_p0/eager_agg/bilateral_eager_agg.groovy
index 76c1cc5f1d7..d59fc4e1896 100644
--- a/regression-test/suites/query_p0/eager_agg/bilateral_eager_agg.groovy
+++ b/regression-test/suites/query_p0/eager_agg/bilateral_eager_agg.groovy
@@ -950,4 +950,78 @@ suite("bilateral_eager_agg") {
WHERE l.filter_date = '2018-01-08'
GROUP BY group_flag;
"""
+
+ multi_sql """
+ DROP TABLE IF EXISTS src_a;
+ DROP TABLE IF EXISTS src_b;
+ DROP TABLE IF EXISTS src_c;
+
+ CREATE TABLE src_a (
+ k BIGINT NOT NULL,
+ v BIGINT NOT NULL
+ )
+ DUPLICATE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1"
+ );
+
+ CREATE TABLE src_b (
+ k BIGINT NOT NULL,
+ join_id BIGINT NOT NULL
+ )
+ DUPLICATE KEY(k, join_id)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1"
+ );
+
+ CREATE TABLE src_c (
+ join_id BIGINT NOT NULL
+ )
+ DUPLICATE KEY(join_id)
+ DISTRIBUTED BY HASH(join_id) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1"
+ );
+
+ INSERT INTO src_a VALUES
+ (1, 10),
+ (2, 20);
+
+ INSERT INTO src_b VALUES
+ (1, 101),
+ (2, 102);
+
+ INSERT INTO src_c VALUES
+ (101),
+ (102);
+
+ SET disable_join_reorder = true;
+ SET eager_aggregation_mode = 1;
+ SET fe_debug = true;
+ """
+
+ order_qt_union_2_same_agg_func """
+ SELECT
+ u.k,
+ SUM(u.x) AS sum_x,
+ SUM(u.y) AS sum_y
+ FROM (
+ SELECT
+ a.k,
+ a.v AS x,
+ a.v AS y
+ FROM src_a a
+ UNION ALL
+ SELECT
+ b.k,
+ CAST(0 AS BIGINT) AS x,
+ CAST(0 AS BIGINT) AS y
+ FROM src_b b
+ INNER JOIN src_c c
+ ON b.join_id = c.join_id
+ ) u
+ GROUP BY u.k;
+ """
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
