github-actions[bot] commented on code in PR #65537:
URL: https://github.com/apache/doris/pull/65537#discussion_r3578167347
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -173,6 +173,10 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan>
aggregate, Optional<Logi
ImmutableSet.Builder<Expression> needPushDownSelfExprs =
ImmutableSet.builder();
ImmutableSet.Builder<Expression> needPushDownInputs =
ImmutableSet.builder();
for (AggregateFunction aggFunc : aggFuncs.keySet()) {
+ if (ExpressionUtils.hasNonWindowAggregateFunction(aggFunc)) {
Review Comment:
This is now checking the aggregate function node itself, not just its
arguments. `aggFuncs.keySet()` contains the collected `AggregateFunction`
roots, and `ExpressionUtils.hasNonWindowAggregateFunction(Expression)` returns
true as soon as the visitor reaches an `AggregateFunction`
(`visitAggregateFunction` returns `true`). So a normal aggregate like `select
sum(k) ...` reaches this line with `aggFunc = sum(k)` and throws `aggregate
function cannot contain aggregate parameters`, even though `k` has no nested
aggregate. Please keep the check before slot normalization, but apply it only
to the original aggregate children/order-key expressions, e.g. the children of
each collected aggregate, so ordinary aggregates still pass while
`group_concat(k order by sum(k))` and similar nested cases fail.
--
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]
