morrySnow commented on code in PR #10659: URL: https://github.com/apache/doris/pull/10659#discussion_r915563444
########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/AggregateDisassemble.java: ########## @@ -64,49 +65,69 @@ public Rule<Plan> build() { 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)); + List<Expression> groupByExpressionList = agg.getGroupByExpressionList(); + + Map<AggregateFunction, NamedExpression> aggregateFunctionAliasMap = Maps.newHashMap(); + for (NamedExpression outputExpression : outputExpressionList) { + outputExpression.foreach(e -> { + if (e instanceof AggregateFunction) { + AggregateFunction a = (AggregateFunction) e; + aggregateFunctionAliasMap.put(a, new Alias<>(a, a.sql())); + } + }); + } + + List<Expression> updateGroupByExpressionList = groupByExpressionList; + List<NamedExpression> updateGroupByAliasList = updateGroupByExpressionList.stream() + .map(g -> new Alias<>(g, g.sql())) + .collect(Collectors.toList()); + + List<NamedExpression> updateOutputExpressionList = Lists.newArrayList(); + updateOutputExpressionList.addAll(updateGroupByAliasList); + updateOutputExpressionList.addAll(aggregateFunctionAliasMap.values()); + + List<Expression> mergeGroupByExpressionList = updateGroupByAliasList.stream() + .map(NamedExpression::toSlot).collect(Collectors.toList()); + + List<NamedExpression> mergeOutputExpressionList = Lists.newArrayList(); + for (NamedExpression o : outputExpressionList) { + if (o.contains(AggregateFunction.class::isInstance)) { + mergeOutputExpressionList.add((NamedExpression) new AggregateFunctionParamsRewriter() + .visit(o, aggregateFunctionAliasMap)); + } else { + for (int i = 0; i < updateGroupByAliasList.size(); i++) { + // TODO: we need to do sub tree match and replace. but we do not have semanticEquals now. + // e.g. a + 1 + 2 in output expression should be replaced by + // (slot reference to update phase out (a + 1)) + 2, if we do group by a + 1 + // currently, we could only handle output expression same with group by expression + if (o instanceof SlotReference) { + // a in output expression will be SLotReference + if (o.equals(updateGroupByExpressionList.get(i))) { + mergeOutputExpressionList.add(updateGroupByAliasList.get(i).toSlot()); + break; + } + } else if (o instanceof Alias) { + // a + 1 in output expression will be Alias + if (o.child(0).equals(updateGroupByExpressionList.get(i))) { + mergeOutputExpressionList.add(updateGroupByAliasList.get(i).toSlot()); + break; + } + } } } - intermediateAggExpressionList.add(namedExpression); } + LogicalAggregate localAgg = new LogicalAggregate( - agg.getGroupByExprList().stream().map(Expression::clone).collect(Collectors.toList()), - intermediateAggExpressionList, + updateGroupByExpressionList, + updateOutputExpressionList, Review Comment: i use the names in the stale planner. imo, local and global is better than update and merge. but as discussion before, we want to reuse names in stale planner as much as possible. so i reserve these names. -- 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