This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new 24d9c831891 branch-4.0: [fix](fe) Normalize aggregate order by
pushdown #64787 (#65026)
24d9c831891 is described below
commit 24d9c831891834859cda63cb7d4d52913b28508c
Author: morrySnow <[email protected]>
AuthorDate: Thu Jul 2 11:44:15 2026 +0800
branch-4.0: [fix](fe) Normalize aggregate order by pushdown #64787 (#65026)
Backport #64787
---
.../nereids/rules/analysis/NormalizeAggregate.java | 10 ++-
.../rules/analysis/NormalizeAggregateTest.java | 78 ++++++++++++++++++++++
.../data/nereids_p0/aggregate/agg_group_concat.out | 9 +++
.../nereids_p0/aggregate/agg_group_concat.groovy | 17 ++++-
4 files changed, 110 insertions(+), 4 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 648b68cccc2..937c499db66 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
@@ -179,7 +179,11 @@ public class NormalizeAggregate implements
RewriteRuleFactory, NormalizeToSlot {
continue;
}
if (arg.containsType(SubqueryExpr.class,
WindowExpression.class, PreferPushDownProject.class)) {
- needPushDownSelfExprs.add(arg);
+ if (arg instanceof OrderExpression) {
+ needPushDownSelfExprs.add(arg.child(0));
+ } else {
+ needPushDownSelfExprs.add(arg);
+ }
} else {
needPushDownInputs.add(arg);
}
@@ -192,8 +196,8 @@ public class NormalizeAggregate implements
RewriteRuleFactory, NormalizeToSlot {
continue;
}
- Collection<? extends Expression> inputSlots
- = arg instanceof OrderExpression ?
arg.getInputSlots() : ImmutableList.of(arg);
+ Collection<? extends Expression> inputSlots = arg
instanceof OrderExpression
+ ? ImmutableList.of(((OrderExpression)
arg).child()) : ImmutableList.of(arg);
for (Expression input : inputSlots) {
if (input instanceof SlotReference) {
needPushDownInputs.add(input);
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 6a66c09b445..8f2f70c3533 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
@@ -23,9 +23,11 @@ import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.Multiply;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.OrderExpression;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.StatementScopeIdGenerator;
+import org.apache.doris.nereids.trees.expressions.WindowExpression;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.Sum;
import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral;
@@ -35,6 +37,7 @@ import
org.apache.doris.nereids.trees.plans.logical.LogicalApply;
import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.util.ExpressionUtils;
import org.apache.doris.nereids.util.FieldChecker;
import org.apache.doris.nereids.util.LogicalPlanBuilder;
import org.apache.doris.nereids.util.MemoPatternMatchSupported;
@@ -689,8 +692,83 @@ public class NormalizeAggregateTest extends
TestWithFeService implements MemoPat
);
}
+ @Test
+ public void testAggregateOrderByExpressionNeedPushDown() {
+ String windowSql = "select group_concat(k order by row_number()
over(order by k)) as s "
+ + "from (select 1 as k union all select 2) t";
+ PlanChecker.from(connectContext)
+ .analyze(windowSql)
+ .matchesFromRoot(
+ logicalResultSink(
+ logicalProject(
+ logicalAggregate(
+ logicalProject().when(project
-> {
+
Assertions.assertTrue(ExpressionUtils.containsTypes(
+
project.getProjects(), WindowExpression.class));
+
Assertions.assertFalse(containsExpressionInstance(
+
project.getProjects(), OrderExpression.class));
+ return true;
+ })
+ ).when(agg ->
ExpressionUtils.containsTypes(
+ agg.getOutputExpressions(),
OrderExpression.class))
+ )
+ )
+ );
+
+ String subquerySql = "select group_concat(id order by (select 1)) from
t1";
+ PlanChecker.from(connectContext)
+ .analyze(subquerySql)
+ .matchesFromRoot(
+ logicalResultSink(
+ logicalProject(
+ logicalAggregate(
+ logicalProject().when(project
-> {
+
Assertions.assertFalse(containsExpressionInstance(
+
project.getProjects(), OrderExpression.class));
+ return true;
+ })
+ ).when(agg ->
ExpressionUtils.containsTypes(
+ agg.getOutputExpressions(),
OrderExpression.class))
+ )
+ )
+ );
+ }
+
+ @Test
+ public void testDistinctAggregateOrderByExpressionNeedPushDown() {
+ String sql = "select group_concat(distinct name order by id + no) from
t1";
+ PlanChecker.from(connectContext)
+ .analyze(sql)
+ .matchesFromRoot(
+ logicalResultSink(
+ logicalProject(
+ logicalAggregate(
+ logicalProject().when(project
-> {
+
Assertions.assertTrue(ExpressionUtils.containsTypes(
+
project.getProjects(), Add.class));
+
Assertions.assertFalse(containsExpressionInstance(
+
project.getProjects(), OrderExpression.class));
+ return true;
+ })
+ ).when(agg ->
ExpressionUtils.containsTypes(
+ agg.getOutputExpressions(),
OrderExpression.class))
+ )
+ )
+ );
+ }
+
private void checkExprsToSql(Collection<? extends Expression> expressions,
String... exprsToSql) {
Assertions.assertEquals(Arrays.asList(exprsToSql),
expressions.stream().map(Expression::toSql).collect(Collectors.toList()));
}
+
+ private boolean containsExpressionInstance(Collection<? extends
Expression> expressions,
+ Class<? extends Expression> clazz) {
+ for (Expression expression : expressions) {
+ if (clazz.isInstance(expression)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git a/regression-test/data/nereids_p0/aggregate/agg_group_concat.out
b/regression-test/data/nereids_p0/aggregate/agg_group_concat.out
index b57409a6192..d26327a1c51 100644
--- a/regression-test/data/nereids_p0/aggregate/agg_group_concat.out
+++ b/regression-test/data/nereids_p0/aggregate/agg_group_concat.out
@@ -1,4 +1,13 @@
-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !group_concat_order_by_window --
+1,2
+
+-- !group_concat_order_by_subquery --
+1,1,1,1,2,2
+
+-- !group_concat_distinct_order_by_expr --
+string1,string2,string3
+
-- !test --
abc,abcd,eee
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 6df485ad9ad..f836796d265 100644
--- a/regression-test/suites/nereids_p0/aggregate/agg_group_concat.groovy
+++ b/regression-test/suites/nereids_p0/aggregate/agg_group_concat.groovy
@@ -102,6 +102,21 @@ suite("agg_group_concat") {
sql """select group_concat(distinct kstr), group_concat(kstr2) from
agg_group_concat_table group by kbint order by 1,2;"""
sql """select multi_distinct_group_concat(kstr), group_concat(kstr2) from
agg_group_concat_table group by kbint order by 1,2;"""
+ order_qt_group_concat_order_by_window """
+ select group_concat(k order by row_number() over(order by k)) as s
+ from (select 1 as k union all select 2) t;
+ """
+
+ order_qt_group_concat_order_by_subquery """
+ select group_concat(kint order by (select 1), kint) as s
+ from agg_group_concat_table;
+ """
+
+ order_qt_group_concat_distinct_order_by_expr """
+ select group_concat(distinct kstr order by kint + kbint) as s
+ from agg_group_concat_table;
+ """
+
sql "drop table if exists test_distinct_multi"
sql """
create table test_distinct_multi(a int, b int, c int, d varchar(10), e
date) distributed by hash(a) properties('replication_num'='1');
@@ -110,4 +125,4 @@ suite("agg_group_concat") {
insert into test_distinct_multi
values(1,2,3,'abc','2024-01-02'),(1,2,4,'abc','2024-01-03'),(2,2,4,'abcd','2024-01-02'),(1,2,3,'abcd','2024-01-04'),(1,2,4,'eee','2024-02-02'),(2,2,4,'abc','2024-01-02');
"""
qt_test "select group_concat( distinct d order by d) from
test_distinct_multi order by 1; "
-}
\ No newline at end of file
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
