This is an automated email from the ASF dual-hosted git repository.
morrySnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new b7931405b35 [fix](aggregate) Check all aggregate function arguments
(#65537)
b7931405b35 is described below
commit b7931405b350957fb009090081914526fe7b9111
Author: morrySnow <[email protected]>
AuthorDate: Fri Jul 17 14:18:32 2026 +0800
[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.
---
.../doris/nereids/rules/analysis/NormalizeAggregate.java | 11 ++++++-----
.../doris/nereids/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 7b5e88060de..aebe40c24d7 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 @@ public class NormalizeAggregate implements
RewriteRuleFactory, NormalizeToSlot {
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 @@ public class NormalizeAggregate implements
RewriteRuleFactory, NormalizeToSlot {
// 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 52d7068db08..37aa9d4913a 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 class NormalizeAggregateTest extends
TestWithFeService implements MemoPat
);
}
+ @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 a05a9edb271..ddafd4f2148 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;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
