morrySnow commented on code in PR #12159:
URL: https://github.com/apache/doris/pull/12159#discussion_r965606605
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/AggregateFunction.java:
##########
@@ -25,13 +25,24 @@
public abstract class AggregateFunction extends BoundFunction {
private DataType intermediate;
+ private final boolean isDistinct;
Review Comment:
need overload equals and hashcode and add ut to ExpressionEqualsTest
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalAggregate.java:
##########
@@ -57,24 +57,32 @@
private final List<NamedExpression> outputExpressions;
private final AggPhase aggPhase;
+ // use for scenes containing distinct agg
+ // 1. If there are LOCAL and GLOBAL phases, global is the final phase
+ // 2. If there are LOCAL, GLOBAL and DISTINCT_LOCAL phases, DISTINCT_LOCAL
is the final phase
+ // 3. If there are LOCAL, GLOBAL, DISTINCT_LOCAL, DISTINCT_GLOBAL phases,
+ // DISTINCT_GLOBAL is the final phase
+ private final boolean isFinalPhase;
+
/**
* Desc: Constructor for LogicalAggregate.
*/
public LogicalAggregate(
List<Expression> groupByExpressions,
List<NamedExpression> outputExpressions,
CHILD_TYPE child) {
- this(groupByExpressions, outputExpressions, false, false,
AggPhase.GLOBAL, child);
+ this(groupByExpressions, outputExpressions, false, false, false,
AggPhase.GLOBAL, child);
Review Comment:
i think the default of finalize should be true and aggphase should set to
local to support one stage aggregate
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/properties/RequestPropertyDeriver.java:
##########
@@ -82,14 +83,17 @@ public Void visitPhysicalAggregate(PhysicalAggregate<?
extends Plan> agg, PlanCo
addToRequestPropertyToChildren(PhysicalProperties.ANY);
return null;
}
-
+ // EXCHANGE NODE does not need to be generated between DISTINCT_LOCAL
and GLOBAL
+ if (agg.getAggPhase() == AggPhase.DISTINCT_LOCAL) {
+ addToRequestPropertyToChildren(PhysicalProperties.ANY);
Review Comment:
we should set the request what the DISTINCT_LOCAL agg want to. in later
stage if the missing enforce helper found that the GLOBAL agg's distribute
satisfy the DISTINCT_LOCAL agg, no exchange node will be added
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/implementation/LogicalAggToPhysicalHashAgg.java:
##########
@@ -36,6 +36,7 @@ public Rule build() {
ImmutableList.of(),
agg.getAggPhase(),
false,
+ agg.isFinalPhase(),
Review Comment:
👍🏻
--
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
