EmmyMiao87 commented on code in PR #10659:
URL: https://github.com/apache/doris/pull/10659#discussion_r916493625


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java:
##########
@@ -17,144 +17,132 @@
 
 package org.apache.doris.nereids.rules.rewrite;
 
-import org.apache.doris.analysis.FunctionName;
-import org.apache.doris.catalog.Catalog;
-import org.apache.doris.catalog.Function;
-import org.apache.doris.catalog.Function.CompareMode;
-import org.apache.doris.catalog.Type;
-import org.apache.doris.nereids.operators.Operator;
 import org.apache.doris.nereids.operators.plans.AggPhase;
 import org.apache.doris.nereids.operators.plans.logical.LogicalAggregate;
 import org.apache.doris.nereids.rules.Rule;
 import org.apache.doris.nereids.rules.RuleType;
+import org.apache.doris.nereids.trees.expressions.Alias;
 import org.apache.doris.nereids.trees.expressions.Expression;
 import org.apache.doris.nereids.trees.expressions.NamedExpression;
-import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.expressions.SlotReference;
 import org.apache.doris.nereids.trees.expressions.functions.AggregateFunction;
+import 
org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter;
+import org.apache.doris.nereids.trees.plans.GroupPlan;
 import org.apache.doris.nereids.trees.plans.Plan;
-import org.apache.doris.nereids.types.DataType;
+import org.apache.doris.nereids.trees.plans.logical.LogicalUnaryPlan;
 
-import com.clearspring.analytics.util.Lists;
-import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
 /**
- * TODO: if instance count is 1, shouldn't disassemble the agg operator
  * Used to generate the merge agg node for distributed execution.
- * Do this in following steps:
- *  1. clone output expr list, find all agg function
- *  2. set found agg function intermediaType
- *  3. create new child plan rooted at new local agg
- *  4. update the slot referenced by expr of merge agg
- *  5. create plan rooted at merge agg, return it.
+ * If we have a query: SELECT SUM(v) + 1 FROM t GROUP BY k + 1
+ * the initial plan is:
+ *   Aggregate(phase: [GLOBAL], outputExpr: [SUM(v1 * v2) + 1], groupByExpr: 
[k + 1])
+ *   +-- childPlan
+ * we should rewrite to:
+ *   Aggregate(phase: [GLOBAL], outputExpr: [SUM(a) + 1], groupByExpr: [b])
+ *   +-- Aggregate(phase: [LOCAL], outputExpr: [SUM(v1 * v2) as a, (k + 1) as 
b], groupByExpr: [k + 1])
+ *       +-- childPlan
+ *
+ * TODO:
+ *     1. use different class represent different phase aggregate
+ *     2. if instance count is 1, shouldn't disassemble the agg operator
+ *     3. we need another rule to removing duplicated expressions in group by 
expression list
  */
 public class AggregateDisassemble extends OneRewriteRuleFactory {
 
     @Override
     public Rule<Plan> build() {
         return logicalAggregate().when(p -> {
-            LogicalAggregate logicalAggregation = p.getOperator();
-            return !logicalAggregation.isDisassembled();
+            LogicalAggregate logicalAggregate = p.getOperator();
+            return !logicalAggregate.isDisassembled();
         }).thenApply(ctx -> {
-            Plan plan = ctx.root;
-            Operator operator = plan.getOperator();
-            LogicalAggregate agg = (LogicalAggregate) operator;
-            List<NamedExpression> outputExpressionList = 
agg.getOutputExpressionList();
-            List<NamedExpression> intermediateAggExpressionList = 
Lists.newArrayList();
-            // TODO: shouldn't extract agg function from this field.
-            for (NamedExpression namedExpression : outputExpressionList) {
-                namedExpression = (NamedExpression) namedExpression.clone();
-                List<AggregateFunction> functionCallList =
-                        
namedExpression.collect(org.apache.doris.catalog.AggregateFunction.class::isInstance);
-                // TODO: we will have another mechanism to get corresponding 
stale agg func.
-                for (AggregateFunction functionCall : functionCallList) {
-                    org.apache.doris.catalog.AggregateFunction staleAggFunc = 
findAggFunc(functionCall);
-                    Type staleIntermediateType = 
staleAggFunc.getIntermediateType();
-                    Type staleRetType = staleAggFunc.getReturnType();
-                    if (staleIntermediateType != null && 
!staleIntermediateType.equals(staleRetType)) {
-                        
functionCall.setIntermediate(DataType.convertFromCatalogDataType(staleIntermediateType));
+            LogicalUnaryPlan<LogicalAggregate, GroupPlan> plan = ctx.root;
+            LogicalAggregate aggregate = plan.getOperator();
+            List<NamedExpression> originOutputExprs = 
aggregate.getOutputExpressionList();
+            List<Expression> originGroupByExprs = 
aggregate.getGroupByExpressionList();
+
+            Map<AggregateFunction, NamedExpression> 
originAggregateFunctionWithAlias = Maps.newHashMap();
+            for (NamedExpression originOutputExpr : originOutputExprs) {

Review Comment:
   Keep todo in here



-- 
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

Reply via email to