This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 5dbdb8aa937 branch-4.1: [refactor](fe) Create PreAggInfoContext
top-down from aggregate instead of bottom-up from scan in SetPreAggStatus
#65237 (#65372)
5dbdb8aa937 is described below
commit 5dbdb8aa93787beb0a8bc215fa4cf4ab1fba3f17
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Jul 13 18:57:21 2026 +0800
branch-4.1: [refactor](fe) Create PreAggInfoContext top-down from aggregate
instead of bottom-up from scan in SetPreAggStatus #65237 (#65372)
Cherry-picked from #65237
Co-authored-by: starocean999 <[email protected]>
---
.../nereids/rules/rewrite/SetPreAggStatus.java | 46 +++++++++++++++-------
.../nereids_rules_p0/set_preagg/set_preagg.groovy | 25 +++++++++++-
2 files changed, 54 insertions(+), 17 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
index 9b152efbb20..aef5f660056 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java
@@ -54,10 +54,13 @@ import
org.apache.doris.nereids.trees.plans.visitor.CustomRewriter;
import org.apache.doris.nereids.trees.plans.visitor.DefaultPlanRewriter;
import org.apache.doris.nereids.util.ExpressionUtils;
import org.apache.doris.qe.ConnectContext;
+import org.apache.doris.qe.SessionVariable;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import java.util.ArrayList;
import java.util.HashMap;
@@ -75,6 +78,7 @@ import java.util.stream.Collectors;
*/
public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.PreAggInfoContext>>
implements CustomRewriter {
+ private static final Logger LOG =
LogManager.getLogger(SetPreAggStatus.class);
private Map<RelationId, PreAggInfoContext> olapScanPreAggContexts = new
HashMap<>();
/**
@@ -141,8 +145,11 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
@Override
public Plan visit(Plan plan, Stack<PreAggInfoContext> context) {
+ // push null sentinel to stop preagg collection for children reached
via generic visitor,
+ // while keeping the aggregate's own frame intact on the stack
+ context.push(null);
Plan newPlan = super.visit(plan, context);
- context.clear();
+ context.pop();
return newPlan;
}
@@ -161,10 +168,9 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
logicalOlapScan.getTable().getName()))) {
return logicalOlapScan.withPreAggStatus(PreAggStatus.on());
} else {
- if (context.empty()) {
- context.push(new PreAggInfoContext());
+ if (!context.empty() && context.peek() != null) {
+
context.peek().addRelationId(logicalOlapScan.getRelationId());
}
- context.peek().addRelationId(logicalOlapScan.getRelationId());
return logicalOlapScan;
}
} else {
@@ -175,7 +181,7 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
@Override
public Plan visitLogicalFilter(LogicalFilter<? extends Plan>
logicalFilter, Stack<PreAggInfoContext> context) {
LogicalFilter plan = (LogicalFilter) super.visit(logicalFilter,
context);
- if (!context.empty()) {
+ if (!context.empty() && context.peek() != null) {
context.peek().addFilterConjuncts(plan.getExpressions());
}
return plan;
@@ -185,7 +191,7 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
public Plan visitLogicalJoin(LogicalJoin<? extends Plan, ? extends Plan>
logicalJoin,
Stack<PreAggInfoContext> context) {
LogicalJoin plan = (LogicalJoin) super.visit(logicalJoin, context);
- if (!context.empty()) {
+ if (!context.empty() && context.peek() != null) {
context.peek().addJoinInfo(plan);
}
return plan;
@@ -195,7 +201,7 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
public Plan visitLogicalProject(LogicalProject<? extends Plan>
logicalProject,
Stack<PreAggInfoContext> context) {
LogicalProject plan = (LogicalProject) super.visit(logicalProject,
context);
- if (!context.empty()) {
+ if (!context.empty() && context.peek() != null) {
context.peek().setReplaceMap(plan.getAliasToProducer());
}
return plan;
@@ -204,23 +210,33 @@ public class SetPreAggStatus extends
DefaultPlanRewriter<Stack<SetPreAggStatus.P
@Override
public Plan visitLogicalAggregate(LogicalAggregate<? extends Plan>
logicalAggregate,
Stack<PreAggInfoContext> context) {
+ PreAggInfoContext preAggInfoContext = new PreAggInfoContext();
+ context.push(preAggInfoContext);
Plan plan = super.visit(logicalAggregate, context);
- if (!context.isEmpty()) {
- PreAggInfoContext preAggInfoContext = context.pop();
-
preAggInfoContext.olapScanIds.retainAll(logicalAggregate.child().getInputRelations());
-
preAggInfoContext.addAggregateFunctions(logicalAggregate.getAggregateFunctions());
-
preAggInfoContext.addGroupByExpresssions(logicalAggregate.getGroupByExpressions());
- for (RelationId id : preAggInfoContext.olapScanIds) {
- olapScanPreAggContexts.put(id, preAggInfoContext);
+ PreAggInfoContext popped = context.pop();
+ if (popped != preAggInfoContext) {
+ if (SessionVariable.isFeDebug()) {
+ Preconditions.checkState(popped == preAggInfoContext,
+ "PreAggInfoContext stack mismatch in
visitLogicalAggregate");
+ } else {
+ LOG.warn("PreAggInfoContext stack mismatch in
visitLogicalAggregate: "
+ + "expected {} but got {}. Skipping preagg for this
aggregate.",
+ preAggInfoContext, popped);
+ return plan;
}
}
+ popped.addAggregateFunctions(logicalAggregate.getAggregateFunctions());
+
popped.addGroupByExpresssions(logicalAggregate.getGroupByExpressions());
+ for (RelationId id : popped.olapScanIds) {
+ olapScanPreAggContexts.put(id, popped);
+ }
return plan;
}
@Override
public Plan visitLogicalRepeat(LogicalRepeat<? extends Plan> repeat,
Stack<PreAggInfoContext> context) {
repeat = (LogicalRepeat<? extends Plan>) super.visit(repeat, context);
- if (!context.isEmpty()) {
+ if (!context.isEmpty() && context.peek() != null) {
context.peek().addGroupingScalarFunctionExpresssion(repeat.getGroupingId().get());
context.peek().addGroupingScalarFunctionExpresssions(
repeat.getOutputExpressions().stream()
diff --git
a/regression-test/suites/nereids_rules_p0/set_preagg/set_preagg.groovy
b/regression-test/suites/nereids_rules_p0/set_preagg/set_preagg.groovy
index 888892bc9c4..f47b2b40fc2 100644
--- a/regression-test/suites/nereids_rules_p0/set_preagg/set_preagg.groovy
+++ b/regression-test/suites/nereids_rules_p0/set_preagg/set_preagg.groovy
@@ -271,7 +271,7 @@ suite("set_preagg") {
""")
contains "(preagg_t1), PREAGGREGATION: ON"
contains "(preagg_t2), PREAGGREGATION: ON"
- contains "(preagg_t3), PREAGGREGATION: OFF"
+ contains "(preagg_t3), PREAGGREGATION: ON"
}
explain {
@@ -291,7 +291,7 @@ suite("set_preagg") {
""")
contains "(preagg_t1), PREAGGREGATION: ON"
contains "(preagg_t2), PREAGGREGATION: ON"
- contains "(preagg_t3), PREAGGREGATION: OFF"
+ contains "(preagg_t3), PREAGGREGATION: ON"
}
explain {
@@ -328,4 +328,25 @@ suite("set_preagg") {
contains "(preagg_t1), PREAGGREGATION: OFF. Reason: No valid aggregate
on scan."
contains "(preagg_t1), PREAGGREGATION: ON"
}
+
+ // Aggregate over limited subquery: Limit between aggregate and scan goes
+ // through generic visitor path, which should block preagg collection
+ // without clearing the aggregate's own frame.
+ explain {
+ sql("""
+ select k1, sum(v7)
+ from (select k1, v7 from preagg_t1 limit 10) s
+ group by k1;
+ """)
+ notContains "(preagg_t1), PREAGGREGATION: ON"
+ }
+
+ // Aggregate over non-Olap relation (numbers TVF).
+ // The TVF scan goes through the generic visitor path, which should not
+ // clear the aggregate's frame and cause EmptyStackException.
+ explain {
+ sql("""
+ select count(*) from (select * from numbers("number"="10")) t;
+ """)
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
