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 5c0adca2c84 [Fix](nereids) fix merge aggregate rule, rules should not
have mutable members (#36145)
5c0adca2c84 is described below
commit 5c0adca2c846f86a0c5b34d328c67d3c58c276e4
Author: feiniaofeiafei <[email protected]>
AuthorDate: Thu Jun 13 10:32:31 2024 +0800
[Fix](nereids) fix merge aggregate rule, rules should not have mutable
members (#36145)
This bug is induced by #31811.
The innerAggExprIdToAggFunc was a member of MergeAggregate, which was
wrong. Because rules like MergeAggregate are single instances, same
rules applied to different sub-plans will affect each other. This pr
changes innerAggExprIdToAggFunc to a local variable, fixes this bug.
No regression use case was added because it’s not a problem that will
definitely reoccur and requires the same rule to be applied to multiple
plans at the same time.
---
.../doris/nereids/rules/rewrite/MergeAggregate.java | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java
index 23fbd978656..4b9e745ee20 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeAggregate.java
@@ -52,7 +52,6 @@ import java.util.stream.Collectors;
public class MergeAggregate implements RewriteRuleFactory {
private static final ImmutableSet<String> ALLOW_MERGE_AGGREGATE_FUNCTIONS =
ImmutableSet.of("min", "max", "sum", "any_value");
- private Map<ExprId, AggregateFunction> innerAggExprIdToAggFunc = new
HashMap<>();
@Override
public List<Rule> buildRules() {
@@ -75,7 +74,7 @@ public class MergeAggregate implements RewriteRuleFactory {
*/
private Plan mergeTwoAggregate(LogicalAggregate<LogicalAggregate<Plan>>
outerAgg) {
LogicalAggregate<Plan> innerAgg = outerAgg.child();
-
+ Map<ExprId, AggregateFunction> innerAggExprIdToAggFunc =
getInnerAggExprIdToAggFuncMap(innerAgg);
List<NamedExpression> newOutputExpressions =
outerAgg.getOutputExpressions().stream()
.map(e -> rewriteAggregateFunction(e, innerAggExprIdToAggFunc))
.collect(Collectors.toList());
@@ -97,6 +96,7 @@ public class MergeAggregate implements RewriteRuleFactory {
List<NamedExpression> outputExpressions =
outerAgg.getOutputExpressions();
List<NamedExpression> replacedOutputExpressions =
PlanUtils.replaceExpressionByProjections(
project.getProjects(), (List)
outputExpressions);
+ Map<ExprId, AggregateFunction> innerAggExprIdToAggFunc =
getInnerAggExprIdToAggFuncMap(innerAgg);
// rewrite agg function. e.g. max(max)
List<NamedExpression> replacedAggFunc =
replacedOutputExpressions.stream()
.filter(expr -> (expr instanceof Alias) && (expr.child(0)
instanceof AggregateFunction))
@@ -152,10 +152,7 @@ public class MergeAggregate implements RewriteRuleFactory {
private boolean commonCheck(LogicalAggregate<? extends Plan> outerAgg,
LogicalAggregate<Plan> innerAgg,
boolean sameGroupBy, Optional<LogicalProject> projectOptional) {
- innerAggExprIdToAggFunc = innerAgg.getOutputExpressions().stream()
- .filter(expr -> (expr instanceof Alias) && (expr.child(0)
instanceof AggregateFunction))
- .collect(Collectors.toMap(NamedExpression::getExprId, value ->
(AggregateFunction) value.child(0),
- (existValue, newValue) -> existValue));
+ Map<ExprId, AggregateFunction> innerAggExprIdToAggFunc =
getInnerAggExprIdToAggFuncMap(innerAgg);
Set<AggregateFunction> aggregateFunctions =
outerAgg.getAggregateFunctions();
List<AggregateFunction> replacedAggFunctions =
projectOptional.map(project ->
(List<AggregateFunction>)
PlanUtils.replaceExpressionByProjections(
@@ -225,4 +222,11 @@ public class MergeAggregate implements RewriteRuleFactory {
boolean sameGroupBy = (innerAgg.getGroupByExpressions().size() ==
outerAgg.getGroupByExpressions().size());
return commonCheck(outerAgg, innerAgg, sameGroupBy,
Optional.of(project));
}
+
+ private Map<ExprId, AggregateFunction>
getInnerAggExprIdToAggFuncMap(LogicalAggregate<Plan> innerAgg) {
+ return innerAgg.getOutputExpressions().stream()
+ .filter(expr -> (expr instanceof Alias) && (expr.child(0)
instanceof AggregateFunction))
+ .collect(Collectors.toMap(NamedExpression::getExprId, value ->
(AggregateFunction) value.child(0),
+ (existValue, newValue) -> existValue));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
