github-actions[bot] commented on code in PR #65537:
URL: https://github.com/apache/doris/pull/65537#discussion_r3569517204
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -258,7 +258,7 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan>
aggregate, Optional<Logi
List<Expression> normalizedAggFuncs =
bottomSlotContext.normalizeToUseSlotRef(SessionVarGuardExpr.getExprWithGuard(aggFuncs));
if (normalizedAggFuncs.stream().anyMatch(agg ->
!agg.children().isEmpty()
- && agg.child(0).containsType(AggregateFunction.class))) {
+ && agg.children().stream().anyMatch(e ->
e.containsType(AggregateFunction.class)))) {
Review Comment:
This check still runs too late for children that were put into
`needPushDownSelfExprs`. For example, `group_concat(distinct k order by
sum(k))` unwraps the order key in the distinct branch and maps `sum(k)` to a
bottom-project alias slot before this block runs; similarly, `count(distinct
sum(k))` or an order key like `sum(k) + row_number() over(...)` can be replaced
by a slot first. After `normalizeToUseSlotRef`, the aggregate child no longer
contains an `AggregateFunction`, so this check misses the nested aggregate and
may only fail later as an aggregate pushed into the wrong plan node. Please
validate the original aggregate children/order-key children for non-window
aggregate functions before any slot normalization, and add coverage for the
distinct and pushdown-triggered variants.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
