924060929 commented on code in PR #10659:
URL: https://github.com/apache/doris/pull/10659#discussion_r915532556
##########
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 think this variable names and compute logic is confuse, how about this:
1. localGroupByExprs = originGloupByExprs
2. localOutputExprs = originOutput.withAlias
3. globalGroupByWithAlias = originGloupByExprs.withAlias
4. globalOutputWithAlias =
originOutput.replaceAggregateFunctionArgumentsToAliasReference
this advantage is
1. variableName contains position and member information
2. assign statement contains the most simple compute logical
--
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
