This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-4.0-preview
in repository https://gitbox.apache.org/repos/asf/doris.git
commit d6c1a27eadecfeea337729ed6897ef9fe96fbc84
Author: morrySnow <101034200+morrys...@users.noreply.github.com>
AuthorDate: Thu Apr 25 16:42:35 2024 +0800
[fix](Nereids) support aggregate function only in having statement (#34086)
SQL like
> SELECT 1 AS c1 FROM t HAVING count(1) > 0 OR c1 IS NOT NULL
---
.../nereids/rules/analysis/FillUpMissingSlots.java | 71 +++++++++++++++++-----
.../test_having_with_aggregate_function.out | 4 ++
.../test_having_with_aggregate_function.groovy | 32 ++++++++++
3 files changed, 92 insertions(+), 15 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
index 82de4453c13..1cab3614302 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
@@ -30,6 +30,7 @@ import
org.apache.doris.nereids.trees.expressions.SlotReference;
import
org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.algebra.Aggregate;
+import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalHaving;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.trees.plans.logical.LogicalSort;
@@ -158,20 +159,52 @@ public class FillUpMissingSlots implements
AnalysisRuleFactory {
// Convert having to filter
RuleType.FILL_UP_HAVING_PROJECT.build(
logicalHaving(logicalProject()).then(having -> {
- LogicalProject<Plan> project = having.child();
- Set<Slot> projectOutputSet = project.getOutputSet();
- Set<Slot> notExistedInProject =
having.getExpressions().stream()
- .map(Expression::getInputSlots)
- .flatMap(Set::stream)
- .filter(s -> !projectOutputSet.contains(s))
- .collect(Collectors.toSet());
- if (notExistedInProject.isEmpty()) {
- return null;
+ if (having.getExpressions().stream().anyMatch(e ->
e.containsType(AggregateFunction.class))) {
+ // This is very weird pattern.
+ // There are some aggregate functions in having,
but its child is project.
+ // There are some slot from project in having too.
+ // Having should execute after project.
+ // But aggregate function should execute before
project.
+ // Since no aggregate here, we should add an empty
aggregate before project.
+ // We should push aggregate function into
aggregate node first.
+ // Then put aggregate result slots and original
project slots into new project.
+ // The final plan should be
+ // Having
+ // +-- Project
+ // +-- Aggregate
+ // Since aggregate node have no group by key.
+ // So project should not contain any slot from its
original child.
+ // Or otherwise slot cannot find will be thrown.
+ LogicalProject<Plan> project = having.child();
+ // new an empty agg here
+ LogicalAggregate<Plan> agg = new
LogicalAggregate<>(
+ ImmutableList.of(), ImmutableList.of(),
project.child());
+ // avoid throw exception even if having have slot
from its child.
+ // because we will add a project between having
and project.
+ Resolver resolver = new Resolver(agg, false);
+ having.getConjuncts().forEach(resolver::resolve);
+ agg =
agg.withAggOutput(resolver.getNewOutputSlots());
+ Set<Expression> newConjuncts =
ExpressionUtils.replace(
+ having.getConjuncts(),
resolver.getSubstitution());
+ ImmutableList.Builder<NamedExpression> projects =
ImmutableList.builder();
+
projects.addAll(project.getOutputs()).addAll(agg.getOutput());
+ return new LogicalHaving<>(newConjuncts, new
LogicalProject<>(projects.build(), agg));
+ } else {
+ LogicalProject<Plan> project = having.child();
+ Set<Slot> projectOutputSet =
project.getOutputSet();
+ Set<Slot> notExistedInProject =
having.getExpressions().stream()
+ .map(Expression::getInputSlots)
+ .flatMap(Set::stream)
+ .filter(s -> !projectOutputSet.contains(s))
+ .collect(Collectors.toSet());
+ if (notExistedInProject.isEmpty()) {
+ return null;
+ }
+ List<NamedExpression> projects =
ImmutableList.<NamedExpression>builder()
+
.addAll(project.getProjects()).addAll(notExistedInProject).build();
+ return new
LogicalProject<>(ImmutableList.copyOf(project.getOutput()),
+ having.withChildren(new
LogicalProject<>(projects, project.child())));
}
- List<NamedExpression> projects =
ImmutableList.<NamedExpression>builder()
-
.addAll(project.getProjects()).addAll(notExistedInProject).build();
- return new
LogicalProject<>(ImmutableList.copyOf(project.getOutput()),
- having.withChildren(new
LogicalProject<>(projects, project.child())));
})
)
);
@@ -184,13 +217,19 @@ public class FillUpMissingSlots implements
AnalysisRuleFactory {
private final Map<Expression, Slot> substitution = Maps.newHashMap();
private final List<NamedExpression> newOutputSlots =
Lists.newArrayList();
private final Map<Slot, Expression> outputSubstitutionMap;
+ private final boolean checkSlot;
- Resolver(Aggregate aggregate) {
+ Resolver(Aggregate<?> aggregate, boolean checkSlot) {
outputExpressions = aggregate.getOutputExpressions();
groupByExpressions = aggregate.getGroupByExpressions();
outputSubstitutionMap =
outputExpressions.stream().filter(Alias.class::isInstance)
.collect(Collectors.toMap(NamedExpression::toSlot, alias
-> alias.child(0),
(k1, k2) -> k1));
+ this.checkSlot = checkSlot;
+ }
+
+ Resolver(Aggregate<?> aggregate) {
+ this(aggregate, true);
}
public void resolve(Expression expression) {
@@ -218,7 +257,9 @@ public class FillUpMissingSlots implements
AnalysisRuleFactory {
// We couldn't find the equivalent expression in output
expressions and group-by expressions,
// so we should check whether the expression is valid.
if (expression instanceof SlotReference) {
- throw new AnalysisException(expression.toSql() + " should
be grouped by.");
+ if (checkSlot) {
+ throw new AnalysisException(expression.toSql() + "
should be grouped by.");
+ }
} else if (expression instanceof AggregateFunction) {
if
(checkWhetherNestedAggregateFunctionsExist((AggregateFunction) expression)) {
throw new AnalysisException("Aggregate functions in
having clause can't be nested: "
diff --git
a/regression-test/data/nereids_rules_p0/fill_up_missing_slots/test_having_with_aggregate_function.out
b/regression-test/data/nereids_rules_p0/fill_up_missing_slots/test_having_with_aggregate_function.out
new file mode 100644
index 00000000000..2bef87ab2b7
--- /dev/null
+++
b/regression-test/data/nereids_rules_p0/fill_up_missing_slots/test_having_with_aggregate_function.out
@@ -0,0 +1,4 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !having_project_with_having_count_1_and_slot_from_project --
+1
+
diff --git
a/regression-test/suites/nereids_rules_p0/fill_up_missing_slots/test_having_with_aggregate_function.groovy
b/regression-test/suites/nereids_rules_p0/fill_up_missing_slots/test_having_with_aggregate_function.groovy
new file mode 100644
index 00000000000..9047a7c85bf
--- /dev/null
+++
b/regression-test/suites/nereids_rules_p0/fill_up_missing_slots/test_having_with_aggregate_function.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("test_having_project") {
+ sql "SET enable_nereids_planner=true"
+ sql "SET enable_fallback_to_original_planner=false"
+
+ sql """
+ DROP TABLE IF EXISTS t
+ """
+ sql """
+ create table t(id smallint) distributed by random
properties('replication_num'='1');
+ """
+
+ qt_having_project_with_having_count_1_and_slot_from_project """
+ SELECT 1 AS c1 FROM t HAVING count(1) > 0 OR c1 IS NOT NULL
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
