morrySnow commented on code in PR #49589:
URL: https://github.com/apache/doris/pull/49589#discussion_r2030431677
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -389,4 +400,88 @@ private Expression
normalizeAggFuncChildren(NormalizeToSlotContext context, Expr
return expr;
}
}
+
+ private LogicalProject<Plan>
eliminateGroupByConstant(NormalizeToSlotContext groupByExprContext,
+ ExpressionRewriteContext rewriteContext, List<Expression>
normalizedGroupExprs,
+ List<NamedExpression> normalizedAggOutput, Set<NamedExpression>
bottomProjects,
+ LogicalAggregate<Plan> aggregate, List<NamedExpression>
upperProjects) {
+ // 1. Find the expressions in group by that can be folded into
constants and build a map(slot, literal)
+ Map<Expression, NormalizeToSlotTriplet> replaceMap =
groupByExprContext.getNormalizeToSlotMap();
+ if (replaceMap.isEmpty()) {
+ return null;
+ }
+ Map<Slot, Expression> slotToLiteral = new HashMap<>();
+ for (Map.Entry<Expression, NormalizeToSlotTriplet> entry :
replaceMap.entrySet()) {
+ Expression foldExpression =
FoldConstantRuleOnFE.evaluate(entry.getKey(), rewriteContext);
+ if (foldExpression.isConstant()) {
+ slotToLiteral.put(entry.getValue().remainExpr, foldExpression);
+ }
+ }
+ if (slotToLiteral.isEmpty()) {
+ return null;
+ }
+ // 2. Regenerate a group by list without constant key
+ List<Expression> newNormalizedGroupExprs = new ArrayList<>();
+ Expression lit = null;
+ for (Expression normalizedGroupExpr : normalizedGroupExprs) {
+ if (!slotToLiteral.containsKey((Slot) normalizedGroupExpr)) {
+ newNormalizedGroupExprs.add(normalizedGroupExpr);
+ } else {
+ lit = normalizedGroupExpr;
+ }
+ }
+ if (newNormalizedGroupExprs.isEmpty() && lit != null) {
+ newNormalizedGroupExprs.add(lit);
+ slotToLiteral.remove(lit);
+ }
+ if (slotToLiteral.isEmpty() || newNormalizedGroupExprs.size() ==
normalizedGroupExprs.size()) {
+ return null;
+ }
+ // 3. Replace the agg output expression and delete the constant group
by key in the output
+ List<NamedExpression> nonConstAggOutput = new ArrayList<>();
+ for (NamedExpression ne : normalizedAggOutput) {
+ if (ne instanceof Alias) {
+
nonConstAggOutput.add(ExpressionUtils.replaceNameExpression(ne, slotToLiteral));
+ continue;
+ } else if (ne instanceof Slot) {
+ if (!slotToLiteral.containsKey(ne)) {
+ nonConstAggOutput.add(ne);
+ }
+ continue;
+ }
+ nonConstAggOutput.add(ne);
+ }
+
+ // The constant expression calculation in bottom projects needs to be
deleted
+ // and put into upperProjects for calculation
+ Plan bottomPlan;
+ if (!bottomProjects.isEmpty()) {
+ List<NamedExpression> newBottomProjects = bottomProjects.stream()
+ .filter(expr -> !slotToLiteral.containsKey(expr.toSlot()))
+ .collect(Collectors.toList());
Review Comment:
collect to immutable list
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -389,4 +400,88 @@ private Expression
normalizeAggFuncChildren(NormalizeToSlotContext context, Expr
return expr;
}
}
+
+ private LogicalProject<Plan>
eliminateGroupByConstant(NormalizeToSlotContext groupByExprContext,
Review Comment:
could we test this method in UT?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -389,4 +400,88 @@ private Expression
normalizeAggFuncChildren(NormalizeToSlotContext context, Expr
return expr;
}
}
+
+ private LogicalProject<Plan>
eliminateGroupByConstant(NormalizeToSlotContext groupByExprContext,
+ ExpressionRewriteContext rewriteContext, List<Expression>
normalizedGroupExprs,
+ List<NamedExpression> normalizedAggOutput, Set<NamedExpression>
bottomProjects,
+ LogicalAggregate<Plan> aggregate, List<NamedExpression>
upperProjects) {
+ // 1. Find the expressions in group by that can be folded into
constants and build a map(slot, literal)
+ Map<Expression, NormalizeToSlotTriplet> replaceMap =
groupByExprContext.getNormalizeToSlotMap();
+ if (replaceMap.isEmpty()) {
+ return null;
+ }
+ Map<Slot, Expression> slotToLiteral = new HashMap<>();
+ for (Map.Entry<Expression, NormalizeToSlotTriplet> entry :
replaceMap.entrySet()) {
+ Expression foldExpression =
FoldConstantRuleOnFE.evaluate(entry.getKey(), rewriteContext);
+ if (foldExpression.isConstant()) {
+ slotToLiteral.put(entry.getValue().remainExpr, foldExpression);
+ }
+ }
+ if (slotToLiteral.isEmpty()) {
+ return null;
+ }
+ // 2. Regenerate a group by list without constant key
+ List<Expression> newNormalizedGroupExprs = new ArrayList<>();
+ Expression lit = null;
+ for (Expression normalizedGroupExpr : normalizedGroupExprs) {
+ if (!slotToLiteral.containsKey((Slot) normalizedGroupExpr)) {
+ newNormalizedGroupExprs.add(normalizedGroupExpr);
+ } else {
+ lit = normalizedGroupExpr;
+ }
+ }
+ if (newNormalizedGroupExprs.isEmpty() && lit != null) {
+ newNormalizedGroupExprs.add(lit);
+ slotToLiteral.remove(lit);
+ }
+ if (slotToLiteral.isEmpty() || newNormalizedGroupExprs.size() ==
normalizedGroupExprs.size()) {
+ return null;
+ }
+ // 3. Replace the agg output expression and delete the constant group
by key in the output
+ List<NamedExpression> nonConstAggOutput = new ArrayList<>();
+ for (NamedExpression ne : normalizedAggOutput) {
+ if (ne instanceof Alias) {
+
nonConstAggOutput.add(ExpressionUtils.replaceNameExpression(ne, slotToLiteral));
+ continue;
+ } else if (ne instanceof Slot) {
+ if (!slotToLiteral.containsKey(ne)) {
+ nonConstAggOutput.add(ne);
+ }
+ continue;
+ }
+ nonConstAggOutput.add(ne);
+ }
+
+ // The constant expression calculation in bottom projects needs to be
deleted
+ // and put into upperProjects for calculation
+ Plan bottomPlan;
+ if (!bottomProjects.isEmpty()) {
+ List<NamedExpression> newBottomProjects = bottomProjects.stream()
+ .filter(expr -> !slotToLiteral.containsKey(expr.toSlot()))
+ .collect(Collectors.toList());
+ bottomPlan = new LogicalProject<>(newBottomProjects,
aggregate.child());
+ } else {
+ bottomPlan = aggregate.child();
+ }
+ LogicalAggregate<Plan> newAggregate =
aggregate.withNormalized(newNormalizedGroupExprs, nonConstAggOutput,
+ bottomPlan);
+ // This upperProjects needs to add the constant key that was deleted
in the group by key
+ // and change the reference to the constant key to a constant
expression
+ List<NamedExpression> newUpperProjects = new ArrayList<>();
Review Comment:
use immutable list
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -279,8 +285,13 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan>
aggregate, Optional<Logi
List<NamedExpression> upperProjects = normalizeOutput(aggregateOutput,
groupByExprContext, argsOfAggFuncNeedPushDownContext,
normalizedAggFuncsToSlotContext);
- // create a parent project node
- LogicalProject<Plan> project = new LogicalProject<>(upperProjects,
newAggregate);
+ ExpressionRewriteContext rewriteContext = new
ExpressionRewriteContext(ctx);
+ LogicalProject<Plan> project =
eliminateGroupByConstant(groupByExprContext, rewriteContext,
+ normalizedGroupExprs, normalizedAggOutput, bottomProjects,
aggregate, upperProjects);
+ if (project == null) {
+ project = new LogicalProject<>(upperProjects, newAggregate);
Review Comment:
should generate newAggregate and bottom project node here
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -175,7 +174,6 @@ public class Rewriter extends AbstractBatchJobExecutor {
topDown(
new EliminateOrderByConstant(),
new EliminateSortUnderSubqueryOrView(),
- new EliminateGroupByConstant(),
Review Comment:
remove EliminateGroupByConstant file
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -279,8 +285,13 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan>
aggregate, Optional<Logi
List<NamedExpression> upperProjects = normalizeOutput(aggregateOutput,
groupByExprContext, argsOfAggFuncNeedPushDownContext,
normalizedAggFuncsToSlotContext);
- // create a parent project node
- LogicalProject<Plan> project = new LogicalProject<>(upperProjects,
newAggregate);
+ ExpressionRewriteContext rewriteContext = new
ExpressionRewriteContext(ctx);
+ LogicalProject<Plan> project =
eliminateGroupByConstant(groupByExprContext, rewriteContext,
+ normalizedGroupExprs, normalizedAggOutput, bottomProjects,
aggregate, upperProjects);
Review Comment:
current code is very weird, because we assemble plan in two different place.
the better impl of is that we always assemble plan in it or out of it.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -389,4 +400,88 @@ private Expression
normalizeAggFuncChildren(NormalizeToSlotContext context, Expr
return expr;
}
}
+
+ private LogicalProject<Plan>
eliminateGroupByConstant(NormalizeToSlotContext groupByExprContext,
+ ExpressionRewriteContext rewriteContext, List<Expression>
normalizedGroupExprs,
+ List<NamedExpression> normalizedAggOutput, Set<NamedExpression>
bottomProjects,
+ LogicalAggregate<Plan> aggregate, List<NamedExpression>
upperProjects) {
+ // 1. Find the expressions in group by that can be folded into
constants and build a map(slot, literal)
+ Map<Expression, NormalizeToSlotTriplet> replaceMap =
groupByExprContext.getNormalizeToSlotMap();
+ if (replaceMap.isEmpty()) {
+ return null;
+ }
+ Map<Slot, Expression> slotToLiteral = new HashMap<>();
+ for (Map.Entry<Expression, NormalizeToSlotTriplet> entry :
replaceMap.entrySet()) {
+ Expression foldExpression =
FoldConstantRuleOnFE.evaluate(entry.getKey(), rewriteContext);
+ if (foldExpression.isConstant()) {
+ slotToLiteral.put(entry.getValue().remainExpr, foldExpression);
+ }
+ }
+ if (slotToLiteral.isEmpty()) {
+ return null;
+ }
+ // 2. Regenerate a group by list without constant key
+ List<Expression> newNormalizedGroupExprs = new ArrayList<>();
+ Expression lit = null;
+ for (Expression normalizedGroupExpr : normalizedGroupExprs) {
+ if (!slotToLiteral.containsKey((Slot) normalizedGroupExpr)) {
+ newNormalizedGroupExprs.add(normalizedGroupExpr);
+ } else {
+ lit = normalizedGroupExpr;
+ }
+ }
+ if (newNormalizedGroupExprs.isEmpty() && lit != null) {
+ newNormalizedGroupExprs.add(lit);
+ slotToLiteral.remove(lit);
+ }
+ if (slotToLiteral.isEmpty() || newNormalizedGroupExprs.size() ==
normalizedGroupExprs.size()) {
+ return null;
+ }
+ // 3. Replace the agg output expression and delete the constant group
by key in the output
+ List<NamedExpression> nonConstAggOutput = new ArrayList<>();
Review Comment:
use immutable list
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -389,4 +400,88 @@ private Expression
normalizeAggFuncChildren(NormalizeToSlotContext context, Expr
return expr;
}
}
+
+ private LogicalProject<Plan>
eliminateGroupByConstant(NormalizeToSlotContext groupByExprContext,
+ ExpressionRewriteContext rewriteContext, List<Expression>
normalizedGroupExprs,
+ List<NamedExpression> normalizedAggOutput, Set<NamedExpression>
bottomProjects,
+ LogicalAggregate<Plan> aggregate, List<NamedExpression>
upperProjects) {
+ // 1. Find the expressions in group by that can be folded into
constants and build a map(slot, literal)
+ Map<Expression, NormalizeToSlotTriplet> replaceMap =
groupByExprContext.getNormalizeToSlotMap();
+ if (replaceMap.isEmpty()) {
+ return null;
+ }
+ Map<Slot, Expression> slotToLiteral = new HashMap<>();
+ for (Map.Entry<Expression, NormalizeToSlotTriplet> entry :
replaceMap.entrySet()) {
+ Expression foldExpression =
FoldConstantRuleOnFE.evaluate(entry.getKey(), rewriteContext);
+ if (foldExpression.isConstant()) {
+ slotToLiteral.put(entry.getValue().remainExpr, foldExpression);
+ }
+ }
+ if (slotToLiteral.isEmpty()) {
+ return null;
+ }
+ // 2. Regenerate a group by list without constant key
+ List<Expression> newNormalizedGroupExprs = new ArrayList<>();
+ Expression lit = null;
+ for (Expression normalizedGroupExpr : normalizedGroupExprs) {
+ if (!slotToLiteral.containsKey((Slot) normalizedGroupExpr)) {
+ newNormalizedGroupExprs.add(normalizedGroupExpr);
+ } else {
+ lit = normalizedGroupExpr;
+ }
+ }
+ if (newNormalizedGroupExprs.isEmpty() && lit != null) {
+ newNormalizedGroupExprs.add(lit);
Review Comment:
maybe add a tinyint 1 is a better way?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
