This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new d646d2c46e0 branch-3.0: [fix](nereids) project distinct to agg checker
exclude window function #54133 (#54541)
d646d2c46e0 is described below
commit d646d2c46e0ef229b5e2eb95f9a66b0fcde144d4
Author: yujun <[email protected]>
AuthorDate: Tue Aug 12 10:45:05 2025 +0800
branch-3.0: [fix](nereids) project distinct to agg checker exclude window
function #54133 (#54541)
cherry pick from #54133
---
.../nereids/rules/analysis/CheckAnalysis.java | 3 +-
.../analysis/OneRowRelationExtractAggregate.java | 25 ++++++----------
.../rules/analysis/ProjectToGlobalAggregate.java | 16 +++--------
.../analysis/ProjectWithDistinctToAggregate.java | 9 ++----
.../apache/doris/nereids/util/ExpressionUtils.java | 20 +++++++++++++
.../project_distinct_to_agg.out | Bin 0 -> 1141 bytes
regression-test/plugins/plugin_planner.groovy | 28 ++++++++++++++++++
.../project_distinct_to_agg.groovy | 32 +++++++++++++++++++++
8 files changed, 96 insertions(+), 37 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java
index ae7bb232e83..e35f420ee7b 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CheckAnalysis.java
@@ -39,6 +39,7 @@ import
org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
import org.apache.doris.nereids.trees.plans.logical.LogicalWindow;
+import org.apache.doris.nereids.util.ExpressionUtils;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -173,7 +174,7 @@ public class CheckAnalysis implements AnalysisRuleFactory {
"The query contains multi count distinct or sum distinct,
each can't have multi columns");
}
for (Expression expr : aggregate.getGroupByExpressions()) {
- if (expr.anyMatch(AggregateFunction.class::isInstance)) {
+ if (ExpressionUtils.hasNonWindowAggregateFunction(expr)) {
throw new AnalysisException(
"GROUP BY expression must not contain aggregate
functions: " + expr.toSql());
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/OneRowRelationExtractAggregate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/OneRowRelationExtractAggregate.java
index 37aaf22ce76..b5eb0649ee0 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/OneRowRelationExtractAggregate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/OneRowRelationExtractAggregate.java
@@ -19,16 +19,13 @@ package org.apache.doris.nereids.rules.analysis;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
-import org.apache.doris.nereids.trees.expressions.NamedExpression;
-import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitors;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalOneRowRelation;
import org.apache.doris.nereids.trees.plans.logical.LogicalRelation;
+import org.apache.doris.nereids.util.ExpressionUtils;
import com.google.common.collect.ImmutableList;
-import java.util.List;
-
/**
* OneRowRelationExtractAggregate.
* <p>
@@ -50,19 +47,13 @@ public class OneRowRelationExtractAggregate extends
OneAnalysisRuleFactory {
@Override
public Rule build() {
return RuleType.ONE_ROW_RELATION_EXTRACT_AGGREGATE.build(
- logicalOneRowRelation().then(relation -> {
- List<NamedExpression> outputs = relation.getOutputs();
- boolean needGlobalAggregate = outputs
- .stream()
- .anyMatch(p ->
p.accept(ExpressionVisitors.CONTAINS_AGGREGATE_CHECKER, null));
- if (needGlobalAggregate) {
- LogicalRelation newRelation = new
LogicalOneRowRelation(relation.getRelationId(),
- ImmutableList.of());
- return new LogicalAggregate<>(ImmutableList.of(),
relation.getOutputs(), newRelation);
- } else {
- return relation;
- }
- })
+ logicalOneRowRelation()
+ .when(relation ->
ExpressionUtils.hasNonWindowAggregateFunction(relation.getOutputs()))
+ .then(relation -> {
+ LogicalRelation newRelation = new
LogicalOneRowRelation(relation.getRelationId(),
+ ImmutableList.of());
+ return new LogicalAggregate<>(ImmutableList.of(),
relation.getOutputs(), newRelation);
+ })
);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectToGlobalAggregate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectToGlobalAggregate.java
index da642e76610..a600bf383d1 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectToGlobalAggregate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectToGlobalAggregate.java
@@ -19,8 +19,8 @@ package org.apache.doris.nereids.rules.analysis;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
-import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitors;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
+import org.apache.doris.nereids.util.ExpressionUtils;
import com.google.common.collect.ImmutableList;
@@ -43,17 +43,9 @@ public class ProjectToGlobalAggregate extends
OneAnalysisRuleFactory {
@Override
public Rule build() {
return RuleType.PROJECT_TO_GLOBAL_AGGREGATE.build(
- logicalProject().then(project -> {
- boolean needGlobalAggregate = project.getProjects()
- .stream()
- .anyMatch(p ->
p.accept(ExpressionVisitors.CONTAINS_AGGREGATE_CHECKER, null));
-
- if (needGlobalAggregate) {
- return new LogicalAggregate<>(ImmutableList.of(),
project.getProjects(), project.child());
- } else {
- return project;
- }
- })
+ logicalProject()
+ .when(project ->
ExpressionUtils.hasNonWindowAggregateFunction(project.getProjects()))
+ .then(project -> new LogicalAggregate<>(ImmutableList.of(),
project.getProjects(), project.child()))
);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectWithDistinctToAggregate.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectWithDistinctToAggregate.java
index f858820d612..804e29f48c1 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectWithDistinctToAggregate.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ProjectWithDistinctToAggregate.java
@@ -19,10 +19,9 @@ package org.apache.doris.nereids.rules.analysis;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
-import org.apache.doris.nereids.trees.expressions.Expression;
-import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
+import org.apache.doris.nereids.util.ExpressionUtils;
/**
* ProjectWithDistinctToAggregate.
@@ -46,12 +45,8 @@ public class ProjectWithDistinctToAggregate extends
OneAnalysisRuleFactory {
return RuleType.PROJECT_WITH_DISTINCT_TO_AGGREGATE.build(
logicalProject()
.when(LogicalProject::isDistinct)
- .whenNot(project ->
project.getProjects().stream().anyMatch(this::hasAggregateFunction))
+ .whenNot(project ->
ExpressionUtils.hasNonWindowAggregateFunction(project.getProjects()))
.then(project -> new LogicalAggregate<>(project.getProjects(),
false, project.child()))
);
}
-
- private boolean hasAggregateFunction(Expression expression) {
- return expression.anyMatch(AggregateFunction.class::isInstance);
- }
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
index fd5f99dfb44..7ec0f287bff 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java
@@ -53,6 +53,7 @@ import
org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
import
org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter;
import
org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionVisitor;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitors;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.logical.LogicalUnion;
import org.apache.doris.nereids.trees.plans.visitor.ExpressionLineageReplacer;
@@ -984,4 +985,23 @@ public class ExpressionUtils {
}
return true;
}
+
+ /**
+ * has aggregate function, exclude the window function
+ */
+ public static boolean hasNonWindowAggregateFunction(Collection<? extends
Expression> expressions) {
+ for (Expression expression : expressions) {
+ if (hasNonWindowAggregateFunction(expression)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * has aggregate function, exclude the window function
+ */
+ public static boolean hasNonWindowAggregateFunction(Expression expression)
{
+ return
expression.accept(ExpressionVisitors.CONTAINS_AGGREGATE_CHECKER, null);
+ }
}
diff --git
a/regression-test/data/nereids_rules_p0/project_distinct_to_agg/project_distinct_to_agg.out
b/regression-test/data/nereids_rules_p0/project_distinct_to_agg/project_distinct_to_agg.out
new file mode 100644
index 00000000000..bba41f2bd61
Binary files /dev/null and
b/regression-test/data/nereids_rules_p0/project_distinct_to_agg/project_distinct_to_agg.out
differ
diff --git a/regression-test/plugins/plugin_planner.groovy
b/regression-test/plugins/plugin_planner.groovy
new file mode 100644
index 00000000000..5b0605da6d8
--- /dev/null
+++ b/regression-test/plugins/plugin_planner.groovy
@@ -0,0 +1,28 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import org.apache.doris.regression.suite.Suite
+
+Suite.metaClass.explainAndResult = { String tag, String sql ->
+ "qt_${tag}_shape" "explain shape plan ${sql}"
+ "qt_${tag}_result" "${sql}"
+}
+
+Suite.metaClass.explainAndOrderResult = { String tag, String sql ->
+ "qt_${tag}_shape" "explain shape plan ${sql}"
+ "order_qt_${tag}_result" "${sql}"
+}
diff --git
a/regression-test/suites/nereids_rules_p0/project_distinct_to_agg/project_distinct_to_agg.groovy
b/regression-test/suites/nereids_rules_p0/project_distinct_to_agg/project_distinct_to_agg.groovy
new file mode 100644
index 00000000000..f6afb9539a9
--- /dev/null
+++
b/regression-test/suites/nereids_rules_p0/project_distinct_to_agg/project_distinct_to_agg.groovy
@@ -0,0 +1,32 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+// OF ANY KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite('project_distinct_to_agg') {
+ def tbl = 'tbl_project_distinct_to_agg'
+ sql "SET ignore_shape_nodes='PhysicalDistribute'"
+ sql "drop table if exists ${tbl} force"
+ sql "create table ${tbl} (a int) distributed by hash(a) buckets 10
properties ('replication_num' = '1')"
+ sql "insert into ${tbl} values (1), (1), (1), (2), (2)"
+
+ explainAndOrderResult 'with_windows_1', "select distinct a, max(a + 10)
over() from ${tbl}"
+ explainAndOrderResult 'with_windows_2', 'select distinct sum(value)
over(partition by id) from (select 100 value, 1 id union all select 100, 2)a'
+ explainAndResult 'order_by', 'select distinct value+1 from (select 100
value, 1 id union all select 100, 2)a order by value+1'
+ explainAndOrderResult 'constant', "select distinct 1, 2, 3 from ${tbl}"
+ explainAndOrderResult 'agg', "select distinct sum(a) from ${tbl}"
+
+ sql "drop table if exists ${tbl} force"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
