From a4ad85013cd35b76ced1b7054c6a7475986c396d Mon Sep 17 00:00:00 2001
From: morrySnow <zhangwenxin@selectdb.com>
Date: Mon, 13 Jul 2026 17:01:21 +0800
Subject: [PATCH] [fix](fe) Check all aggregate function arguments

Issue Number: None

Related PR: None

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.

Reject nested aggregate functions in every aggregate function argument.

- Test: Unit Test and Regression test
    - Unit Test: NormalizeAggregateTest
    - Regression test: query_p0/aggregate/agg_group_concat
- Behavior changed: Yes, invalid nested aggregate functions in non-first arguments are rejected during analysis
- Does this need documentation: No
---
 .../nereids/rules/analysis/NormalizeAggregate.java    | 11 ++++++-----
 .../rules/analysis/NormalizeAggregateTest.java        | 10 ++++++++++
 .../suites/query_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 7b5e88060deded..aebe40c24d7752 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
@@ -173,6 +173,12 @@ 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()) {
+            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
@@ -257,11 +263,6 @@ private LogicalPlan normalizeAgg(LogicalAggregate<Plan> aggregate, Optional<Logi
         // normalize trivial-aggs by bottomProjects
         List<Expression> 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 52d7068db080c4..37aa9d4913a33a 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;
@@ -736,6 +737,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/query_p0/aggregate/agg_group_concat.groovy b/regression-test/suites/query_p0/aggregate/agg_group_concat.groovy
index 861042750c2e85..fc854361bd61e0 100644
--- a/regression-test/suites/query_p0/aggregate/agg_group_concat.groovy
+++ b/regression-test/suites/query_p0/aggregate/agg_group_concat.groovy
@@ -105,6 +105,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;