This is an automated email from the ASF dual-hosted git repository.

kxiao pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-2.0 by this push:
     new 8e405328eea [fix](Nereids) should not put bound expr into unbound 
group by list (#25938) (#26222)
8e405328eea is described below

commit 8e405328eea206f55623f6491543664d32ea2661
Author: morrySnow <[email protected]>
AuthorDate: Wed Nov 1 19:46:29 2023 +0800

    [fix](Nereids) should not put bound expr into unbound group by list 
(#25938) (#26222)
    
    we put bound expr into unbound group by list by mistake.
    This will lead to bind twice on some exprssion.
    Since binding is not idempotent, below exception will be thrown for sql
    
    ```sql
    select k5 / k5 as nu, sum(k1) from test group by nu order by nu nulls first
    ```
    
    ```
    Caused by: org.apache.doris.nereids.exceptions.AnalysisException: Input 
slot(s) not in child's output: k5#5 in plan: LogicalProject[176] ( 
distinct=false, projects=[(cast(k5#5 as DECIMALV3(16, 10)) / k5#5) AS `nu`#14, 
sum(k1)#15], excepts=[] ), child output is: [nu#16, sum(k1)#15]
    plan tree:
    LogicalProject[176] ( distinct=false, projects=[(cast(k5#5 as DECIMALV3(16, 
10)) / k5#5) AS `nu`#14, sum(k1)#15], excepts=[] )
    +--LogicalAggregate[168] ( groupByExpr=[nu#16], outputExpr=[nu#16, 
sum(k1#1) AS `sum(k1)`#15], hasRepeat=false )
       +--LogicalProject[156] ( distinct=false, projects=[k1#1, (cast(k5#5 as 
DECIMALV3(16, 10)) / k5#5) AS `nu`#16], excepts=[] )
          +--LogicalOlapScan ( 
qualified=default_cluster:regression_test_nereids_syntax_p0.test, 
indexName=test, selectedIndexId=503229, preAgg=OFF, Aggregate function sum(k1) 
contains key column k1. )
        at 
org.apache.doris.nereids.rules.analysis.CheckAfterRewrite.checkAllSlotReferenceFromChildren(CheckAfterRewrite.java:108)
 ~[classes/:?]
    ```
---
 .../doris/nereids/rules/analysis/BindExpression.java   | 18 +++++++++++-------
 .../suites/nereids_syntax_p0/analyze_agg.groovy        |  6 +++++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
index 420e6303e0a..7861657301e 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java
@@ -272,13 +272,17 @@ public class BindExpression implements 
AnalysisRuleFactory {
                      group_by_key is bound on t1.a
                     */
                     duplicatedSlotNames.forEach(childOutputsToExpr::remove);
-                    output.stream()
-                            .filter(ne -> ne instanceof Alias)
-                            .map(Alias.class::cast)
-                            // agg function cannot be bound with group_by_key
-                            .filter(alias -> !alias.child()
-                                    .anyMatch(expr -> expr instanceof 
AggregateFunction))
-                            .forEach(alias -> 
childOutputsToExpr.putIfAbsent(alias.getName(), alias.child()));
+                    for (int i = 0; i < output.size(); i++) {
+                        if (!(output.get(i) instanceof Alias)) {
+                            continue;
+                        }
+                        Alias alias = (Alias) output.get(i);
+                        if (alias.child().anyMatch(expr -> expr instanceof 
AggregateFunction)) {
+                            continue;
+                        }
+                        // NOTICE: must use unbound expressions, because we 
will bind them in binding group by expr.
+                        childOutputsToExpr.putIfAbsent(alias.getName(), 
agg.getOutputExpressions().get(i).child(0));
+                    }
 
                     List<Expression> replacedGroupBy = 
agg.getGroupByExpressions().stream()
                             .map(groupBy -> {
diff --git a/regression-test/suites/nereids_syntax_p0/analyze_agg.groovy 
b/regression-test/suites/nereids_syntax_p0/analyze_agg.groovy
index 55bb2deab23..9f426e23e3d 100644
--- a/regression-test/suites/nereids_syntax_p0/analyze_agg.groovy
+++ b/regression-test/suites/nereids_syntax_p0/analyze_agg.groovy
@@ -43,7 +43,8 @@ suite("analyze_agg") {
             d VARCHAR(30),
             e VARCHAR(32),
             a VARCHAR(32),
-            f VARCHAR(32)
+            f VARCHAR(32),
+            g DECIMAL(9, 3)
         )ENGINE = OLAP
         UNIQUE KEY(id)
         DISTRIBUTED BY HASH(id) BUCKETS 30
@@ -73,4 +74,7 @@ suite("analyze_agg") {
         sql "select count(distinct t2.b), variance(distinct t2.c) from t2"
         exception "variance(DISTINCT c#2) can't support multi distinct."
     }
+
+    // should not bind g /g in group by again, otherwise will throw exception
+    sql "select g / g as nu, sum(c) from t2 group by nu"
 }
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to