You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2022/12/12 14:03:45 UTC

[doris] branch master updated: [fix](nereids) having with no group by is not parsed correctly (#14883)

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 281d47434a [fix](nereids) having with no group by is not parsed correctly (#14883)
281d47434a is described below

commit 281d47434a92b6ff89a7c22a2eb7f0c1e1b984e4
Author: starocean999 <40...@users.noreply.github.com>
AuthorDate: Mon Dec 12 22:03:37 2022 +0800

    [fix](nereids) having with no group by is not parsed correctly (#14883)
    
    SQL: SELECT * FROM tbl HAVING c1 > 10;
---
 .../doris/nereids/parser/LogicalPlanBuilder.java   | 26 ++++++++++--
 .../org/apache/doris/nereids/rules/RuleType.java   |  1 +
 .../nereids/rules/analysis/BindSlotReference.java  |  8 ++--
 .../nereids/rules/analysis/FillUpMissingSlots.java |  5 ++-
 regression-test/data/nereids_syntax_p0/having.out  | 46 ++++++++++++++++++++++
 .../suites/nereids_syntax_p0/having.groovy         | 12 ++++++
 6 files changed, 89 insertions(+), 9 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
index 5ee8d839a8..300784cdff 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
@@ -1076,11 +1076,29 @@ public class LogicalPlanBuilder extends DorisParserBaseVisitor<Object> {
             // from -> where -> group by -> having -> select
 
             LogicalPlan filter = withFilter(inputRelation, whereClause);
-            SelectColumnClauseContext columnCtx = selectClause.selectColumnClause();
-            LogicalPlan aggregate = withAggregate(filter, columnCtx, aggClause);
+            SelectColumnClauseContext selectColumnCtx = selectClause.selectColumnClause();
+            LogicalPlan aggregate = withAggregate(filter, selectColumnCtx, aggClause);
             // TODO: replace and process having at this position
-            LogicalPlan having = withHaving(aggregate, havingClause);
-            return withProjection(having, selectClause.selectColumnClause(), aggClause);
+            if (!(aggregate instanceof Aggregate) && havingClause.isPresent()) {
+                // create a project node for pattern match of ProjectToGlobalAggregate rule
+                // then ProjectToGlobalAggregate rule can insert agg node as LogicalHaving node's child
+                LogicalPlan project;
+                if (selectColumnCtx.EXCEPT() != null) {
+                    List<NamedExpression> expressions = getNamedExpressions(selectColumnCtx.namedExpressionSeq());
+                    if (!expressions.stream().allMatch(UnboundSlot.class::isInstance)) {
+                        throw new ParseException("only column name is supported in except clause", selectColumnCtx);
+                    }
+                    project = new LogicalProject<>(ImmutableList.of(new UnboundStar(Collections.emptyList())),
+                        expressions, aggregate);
+                } else {
+                    List<NamedExpression> projects = getNamedExpressions(selectColumnCtx.namedExpressionSeq());
+                    project = new LogicalProject<>(projects, Collections.emptyList(), aggregate);
+                }
+                return new LogicalHaving<>(getExpression((havingClause.get().booleanExpression())), project);
+            } else {
+                LogicalPlan having = withHaving(aggregate, havingClause);
+                return withProjection(having, selectColumnCtx, aggClause);
+            }
         });
     }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
index e3e12f11f5..1ff5b82964 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
@@ -54,6 +54,7 @@ public enum RuleType {
     REPLACE_SORT_EXPRESSION_BY_CHILD_OUTPUT(RuleTypeClass.REWRITE),
 
     FILL_UP_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
+    FILL_UP_HAVING_PROJECT(RuleTypeClass.REWRITE),
     FILL_UP_SORT_AGGREGATE(RuleTypeClass.REWRITE),
     FILL_UP_SORT_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
     FILL_UP_SORT_PROJECT(RuleTypeClass.REWRITE),
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSlotReference.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSlotReference.java
index 12743deb37..853a6cb76d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSlotReference.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSlotReference.java
@@ -246,12 +246,12 @@ public class BindSlotReference implements AnalysisRuleFactory {
                 })
             ),
             RuleType.BINDING_HAVING_SLOT.build(
-                logicalHaving(aggregate()).when(Plan::canBind).thenApply(ctx -> {
-                    LogicalHaving<Aggregate<GroupPlan>> having = ctx.root;
-                    Aggregate<GroupPlan> aggregate = having.child();
+                logicalHaving(any()).when(Plan::canBind).thenApply(ctx -> {
+                    LogicalHaving<Plan> having = ctx.root;
+                    Plan childPlan = having.child();
                     // We should deduplicate the slots, otherwise the binding process will fail due to the
                     // ambiguous slots exist.
-                    Set<Slot> boundSlots = Stream.concat(Stream.of(aggregate), aggregate.children().stream())
+                    Set<Slot> boundSlots = Stream.concat(Stream.of(childPlan), childPlan.children().stream())
                             .flatMap(plan -> plan.getOutput().stream())
                             .collect(Collectors.toSet());
                     Expression boundPredicates = new SlotBinder(
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 3a712d291f..421b19932b 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
@@ -120,7 +120,10 @@ public class FillUpMissingSlots implements AnalysisRuleFactory {
                                 having.getPredicates(), r.getSubstitution());
                         return new LogicalFilter<>(newPredicates, a);
                     });
-                })
+                })),
+            RuleType.FILL_UP_HAVING_PROJECT.build(
+                logicalHaving(logicalProject()).then(having -> new LogicalFilter<>(having.getPredicates(),
+                    having.child()))
             )
         );
     }
diff --git a/regression-test/data/nereids_syntax_p0/having.out b/regression-test/data/nereids_syntax_p0/having.out
index 858c26cdc3..98d530c029 100644
--- a/regression-test/data/nereids_syntax_p0/having.out
+++ b/regression-test/data/nereids_syntax_p0/having.out
@@ -54,3 +54,49 @@
 2
 3
 
+-- !select --
+9
+
+-- !select --
+1
+1
+1
+2
+2
+2
+3
+3
+3
+
+-- !select --
+1
+1
+1
+2
+2
+2
+3
+3
+3
+
+-- !select --
+36
+
+-- !select --
+36
+
+-- !select --
+36
+
+-- !select --
+36
+
+-- !select --
+54
+
+-- !select --
+54
+
+-- !select --
+9
+
diff --git a/regression-test/suites/nereids_syntax_p0/having.groovy b/regression-test/suites/nereids_syntax_p0/having.groovy
index 981230058d..18186bbe5d 100644
--- a/regression-test/suites/nereids_syntax_p0/having.groovy
+++ b/regression-test/suites/nereids_syntax_p0/having.groovy
@@ -18,6 +18,7 @@
 suite("test_nereids_having") {
 
     sql "SET enable_nereids_planner=true"
+    sql "SET enable_fallback_to_original_planner=true"
     sql "SET enable_vectorized_engine=true"
 
     sql "DROP TABLE IF EXISTS test_nereids_having_tbl"
@@ -57,4 +58,15 @@ suite("test_nereids_having") {
     order_qt_select "SELECT a1, SUM(a1 + a2) FROM test_nereids_having_tbl GROUP BY a1 HAVING SUM(a1 + a2) > 0";
     order_qt_select "SELECT a1, SUM(a1 + a2) FROM test_nereids_having_tbl GROUP BY a1 HAVING SUM(a1 + a2 + 3) > 0";
     order_qt_select "SELECT a1 FROM test_nereids_having_tbl GROUP BY a1 HAVING COUNT(*) > 0";
+    order_qt_select "SELECT COUNT(*) FROM test_nereids_having_tbl HAVING COUNT(*) > 0";
+
+    order_qt_select "SELECT a1 as value FROM test_nereids_having_tbl HAVING a1 > 0";
+    order_qt_select "SELECT a1 as value FROM test_nereids_having_tbl HAVING value > 0";
+    order_qt_select "SELECT SUM(a2) FROM test_nereids_having_tbl HAVING SUM(a2) > 0";
+    order_qt_select "SELECT SUM(a2) as value FROM test_nereids_having_tbl HAVING SUM(a2) > 0";
+    order_qt_select "SELECT SUM(a2) as value FROM test_nereids_having_tbl HAVING value > 0";
+    order_qt_select "SELECT SUM(a2) FROM test_nereids_having_tbl HAVING MIN(pk) > 0";
+    order_qt_select "SELECT SUM(a1 + a2) FROM test_nereids_having_tbl HAVING SUM(a1 + a2) > 0";
+    order_qt_select "SELECT SUM(a1 + a2) FROM test_nereids_having_tbl HAVING SUM(a1 + a2 + 3) > 0";
+    order_qt_select "SELECT COUNT(*) FROM test_nereids_having_tbl HAVING COUNT(*) > 0";
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org