From dd110e824caf1e0d0557be3ac60c37accb1e0c4c Mon Sep 17 00:00:00 2001 From: morrySnow Date: Fri, 17 Jul 2026 14:18:32 +0800 Subject: [PATCH] [fix](aggregate) Check all aggregate function arguments (#65537) ### What problem does this PR solve? Problem Summary: NormalizeAggregate only checked the first child of each aggregate function for nested aggregate functions. Aggregate functions in later arguments, such as SUM in the ORDER BY argument of GROUP_CONCAT, could bypass analysis validation. Check every aggregate child and add FE unit and regression coverage for the invalid query. ### Release note Reject nested aggregate functions in every aggregate function argument. --- .../nereids/rules/analysis/NormalizeAggregate.java | 11 ++++++----- .../rules/analysis/NormalizeAggregateTest.java | 10 ++++++++++ .../nereids_p0/aggregate/agg_group_concat.groovy | 8 ++++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java index 937c499db669c2..c9a1c34473fb3f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java @@ -171,6 +171,12 @@ private LogicalPlan normalizeAgg(LogicalAggregate aggregate, Optional needPushDownSelfExprs = ImmutableSet.builder(); ImmutableSet.Builder needPushDownInputs = ImmutableSet.builder(); for (AggregateFunction aggFunc : aggFuncs.keySet()) { + for (Expression child : aggFunc.children()) { + if (ExpressionUtils.hasNonWindowAggregateFunction(child)) { + throw new AnalysisException( + "aggregate function cannot contain aggregate parameters"); + } + } if (!aggFunc.isDistinct()) { for (Expression arg : aggFunc.children()) { // should not push down literal under aggregate @@ -254,11 +260,6 @@ private LogicalPlan normalizeAgg(LogicalAggregate aggregate, Optional normalizedAggFuncs = bottomSlotContext.normalizeToUseSlotRef(SessionVarGuardExpr.getExprWithGuard(aggFuncs)); - if (normalizedAggFuncs.stream().anyMatch(agg -> !agg.children().isEmpty() - && agg.child(0).containsType(AggregateFunction.class))) { - throw new AnalysisException( - "aggregate function cannot contain aggregate parameters"); - } // build normalized agg output NormalizeToSlotContext normalizedAggFuncsToSlotContext = diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregateTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregateTest.java index 8f2f70c3533215..d01fd651cabb39 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregateTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregateTest.java @@ -17,6 +17,7 @@ package org.apache.doris.nereids.rules.analysis; +import org.apache.doris.nereids.exceptions.AnalysisException; import org.apache.doris.nereids.trees.expressions.Add; import org.apache.doris.nereids.trees.expressions.Alias; import org.apache.doris.nereids.trees.expressions.ExprId; @@ -734,6 +735,15 @@ public void testAggregateOrderByExpressionNeedPushDown() { ); } + @Test + public void testAggregateOrderByExpressionCannotContainAggregateFunction() { + String sql = "select group_concat(k order by sum(k)) as s " + + "from (select 1 as k union all select 2) t"; + AnalysisException exception = Assertions.assertThrows(AnalysisException.class, + () -> PlanChecker.from(connectContext).analyze(sql)); + Assertions.assertEquals("aggregate function cannot contain aggregate parameters", exception.getMessage()); + } + @Test public void testDistinctAggregateOrderByExpressionNeedPushDown() { String sql = "select group_concat(distinct name order by id + no) from t1"; diff --git a/regression-test/suites/nereids_p0/aggregate/agg_group_concat.groovy b/regression-test/suites/nereids_p0/aggregate/agg_group_concat.groovy index f836796d265396..cdf5d5292faff5 100644 --- a/regression-test/suites/nereids_p0/aggregate/agg_group_concat.groovy +++ b/regression-test/suites/nereids_p0/aggregate/agg_group_concat.groovy @@ -107,6 +107,14 @@ suite("agg_group_concat") { from (select 1 as k union all select 2) t; """ + test { + sql """ + select group_concat(k order by sum(k)) as s + from (select 1 as k union all select 2) t; + """ + exception "aggregate function cannot contain aggregate parameters" + } + order_qt_group_concat_order_by_subquery """ select group_concat(kint order by (select 1), kint) as s from agg_group_concat_table;