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 479d57df88 [fix](planner) the project expr should be calculated in 
join node in some case (#17035)
479d57df88 is described below

commit 479d57df885c4dfd23093e576fe5482df75b9c6d
Author: starocean999 <40539150+starocean...@users.noreply.github.com>
AuthorDate: Fri Feb 24 15:20:05 2023 +0800

    [fix](planner) the project expr should be calculated in join node in some 
case (#17035)
    
    Consider the sql bellow:
    
    select sum(cc.qlnm) as qlnm
    FROM
      outerjoin_A
      left join (SELECT
          outerjoin_B.b,
          coalesce(outerjoin_C.c, 0) AS qlnm
        FROM
          outerjoin_B
          inner JOIN outerjoin_C ON outerjoin_B.b = outerjoin_C.c
      ) cc on outerjoin_A.a = cc.b
    group by outerjoin_A.a;
    
    The coalesce(outerjoin_C.c, 0) was calculated in the agg node, which is 
wrong.
    This pr correct this, and the expr is calculated in the inner join node now.
---
 .../apache/doris/analysis/ExprSubstitutionMap.java |  4 +
 .../org/apache/doris/planner/JoinNodeBase.java     | 32 ++++++++
 .../java/org/apache/doris/planner/PlanNode.java    |  2 +-
 .../apache/doris/planner/SingleNodePlanner.java    | 15 ++--
 .../test_outer_join_with_inline_view.out           |  3 +
 regression-test/data/query_p0/join/test_join5.out  |  2 +-
 .../test_outer_join_with_inline_view.groovy        | 89 ++++++++++++++++++++++
 7 files changed, 138 insertions(+), 9 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java
index d0f9fadbf7..9423d673b5 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprSubstitutionMap.java
@@ -137,6 +137,10 @@ public final class ExprSubstitutionMap {
         lhs = lhsExprList;
     }
 
+    public void updateRhsExprs(List<Expr> rhsExprList) {
+        rhs = rhsExprList;
+    }
+
     /**
      * Return a map  which is equivalent to applying f followed by g,
      * i.e., g(f()).
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/planner/JoinNodeBase.java 
b/fe/fe-core/src/main/java/org/apache/doris/planner/JoinNodeBase.java
index cfa2865d2a..5d81033362 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/JoinNodeBase.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/JoinNodeBase.java
@@ -586,4 +586,36 @@ public abstract class JoinNodeBase extends PlanNode {
     public void setvSrcToOutputSMap(List<Expr> lhs) {
         this.vSrcToOutputSMap = new ExprSubstitutionMap(lhs, 
Collections.emptyList());
     }
+
+    public void setOutputSmap(ExprSubstitutionMap smap, Analyzer analyzer) {
+        outputSmap = smap;
+        ExprSubstitutionMap tmpSmap = new 
ExprSubstitutionMap(Lists.newArrayList(vSrcToOutputSMap.getRhs()),
+                Lists.newArrayList(vSrcToOutputSMap.getLhs()));
+        List<Expr> newRhs = Lists.newArrayList();
+        boolean bSmapChanged = false;
+        for (Expr expr : smap.getRhs()) {
+            if (expr instanceof SlotRef) {
+                newRhs.add(expr);
+            } else {
+                // we need do project in the join node
+                // add a new slot for projection result and add the project 
expr to vSrcToOutputSMap
+                SlotDescriptor slotDesc = 
analyzer.addSlotDescriptor(vOutputTupleDesc);
+                slotDesc.initFromExpr(expr);
+                slotDesc.setIsMaterialized(true);
+                // the project expr is from smap, which use the slots of hash 
join node's output tuple
+                // we need substitute it to make sure the project expr use 
slots from intermediate tuple
+                expr.substitute(tmpSmap);
+                vSrcToOutputSMap.getLhs().add(expr);
+                SlotRef slotRef = new SlotRef(slotDesc);
+                vSrcToOutputSMap.getRhs().add(slotRef);
+                newRhs.add(slotRef);
+                bSmapChanged = true;
+            }
+        }
+
+        if (bSmapChanged) {
+            outputSmap.updateRhsExprs(newRhs);
+            vOutputTupleDesc.computeStatAndMemLayout();
+        }
+    }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java 
b/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java
index aa57d08bc7..bce032ef07 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/PlanNode.java
@@ -692,7 +692,7 @@ public abstract class PlanNode extends TreeNode<PlanNode> 
implements PlanStats {
         return outputSmap;
     }
 
-    public void setOutputSmap(ExprSubstitutionMap smap) {
+    public void setOutputSmap(ExprSubstitutionMap smap, Analyzer analyzer) {
         outputSmap = smap;
     }
 
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java 
b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
index 44d85396b9..16962e87d8 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/SingleNodePlanner.java
@@ -204,7 +204,7 @@ public class SingleNodePlanner {
         // Set the output smap to resolve exprs referencing inline views 
within stmt.
         // Not needed for a UnionStmt because it materializes its input 
operands.
         if (stmt instanceof SelectStmt) {
-            node.setOutputSmap(((SelectStmt) stmt).getBaseTblSmap());
+            node.setOutputSmap(((SelectStmt) stmt).getBaseTblSmap(), analyzer);
         }
         return node;
     }
@@ -320,7 +320,7 @@ public class SingleNodePlanner {
             scanNodes.removeIf(scanNode -> 
scanTupleIds.contains(scanNode.getTupleIds().get(0)));
             PlanNode node = createEmptyNode(root, stmt, analyzer);
             // Ensure result exprs will be substituted by right outputSmap
-            node.setOutputSmap(root.outputSmap);
+            node.setOutputSmap(root.outputSmap, analyzer);
             return node;
         }
 
@@ -1149,7 +1149,7 @@ public class SingleNodePlanner {
             }
             final PlanNode emptySetNode = new 
EmptySetNode(ctx.getNextNodeId(), rowTuples);
             emptySetNode.init(analyzer);
-            emptySetNode.setOutputSmap(selectStmt.getBaseTblSmap());
+            emptySetNode.setOutputSmap(selectStmt.getBaseTblSmap(), analyzer);
             return createAggregationPlan(selectStmt, analyzer, emptySetNode);
         }
 
@@ -1588,7 +1588,7 @@ public class SingleNodePlanner {
                     // conjuncts into outer-joined inline views, so 
hasEmptyResultSet() cannot be
                     // true for an outer-joined inline view that has no table 
refs.
                     
Preconditions.checkState(!analyzer.isOuterJoined(inlineViewRef.getId()));
-                    emptySetNode.setOutputSmap(inlineViewRef.getSmap());
+                    emptySetNode.setOutputSmap(inlineViewRef.getSmap(), 
analyzer);
                     return emptySetNode;
                 }
                 // Analysis should have generated a tuple id into which to 
materialize the exprs.
@@ -1605,7 +1605,7 @@ public class SingleNodePlanner {
                 unionNode.init(analyzer);
                 //set outputSmap to substitute literal in outputExpr
                 
unionNode.setWithoutTupleIsNullOutputSmap(inlineViewRef.getSmap());
-                unionNode.setOutputSmap(inlineViewRef.getSmap());
+                unionNode.setOutputSmap(inlineViewRef.getSmap(), analyzer);
                 if (analyzer.isOuterJoined(inlineViewRef.getId())) {
                     List<Expr> nullableRhs;
                     if (analyzer.isOuterJoinedLeftSide(inlineViewRef.getId())) 
{
@@ -1615,7 +1615,8 @@ public class SingleNodePlanner {
                         nullableRhs = 
TupleIsNullPredicate.wrapExprs(inlineViewRef.getSmap().getRhs(),
                             unionNode.getTupleIds(), TNullSide.RIGHT, 
analyzer);
                     }
-                    unionNode.setOutputSmap(new 
ExprSubstitutionMap(inlineViewRef.getSmap().getLhs(), nullableRhs));
+                    unionNode.setOutputSmap(
+                            new 
ExprSubstitutionMap(inlineViewRef.getSmap().getLhs(), nullableRhs), analyzer);
                 }
                 return unionNode;
             }
@@ -1646,7 +1647,7 @@ public class SingleNodePlanner {
             outputSmap = new ExprSubstitutionMap(outputSmap.getLhs(), 
nullableRhs);
         }
         // Set output smap of rootNode *before* creating a SelectNode for 
proper resolution.
-        rootNode.setOutputSmap(outputSmap);
+        rootNode.setOutputSmap(outputSmap, analyzer);
         if (rootNode instanceof UnionNode && ((UnionNode) 
rootNode).isConstantUnion()) {
             rootNode.setWithoutTupleIsNullOutputSmap(outputSmap);
         }
diff --git 
a/regression-test/data/correctness_p0/test_outer_join_with_inline_view.out 
b/regression-test/data/correctness_p0/test_outer_join_with_inline_view.out
index 52a9fc4158..4e9d0de8b9 100644
--- a/regression-test/data/correctness_p0/test_outer_join_with_inline_view.out
+++ b/regression-test/data/correctness_p0/test_outer_join_with_inline_view.out
@@ -11,3 +11,6 @@
 3      3       \N      \N
 4      4       \N      \N
 
+-- !select_with_agg_in_inline_view_and_outer_join --
+1
+
diff --git a/regression-test/data/query_p0/join/test_join5.out 
b/regression-test/data/query_p0/join/test_join5.out
index 6ab749f3a7..fb77dad577 100644
--- a/regression-test/data/query_p0/join/test_join5.out
+++ b/regression-test/data/query_p0/join/test_join5.out
@@ -16,7 +16,7 @@
 -- !join5 --
 A      p       2       -1
 B      q       0       -1
-C      \N      0       -1
+C      \N      \N      \N
 
 -- !join5 --
 1      1       1       1
diff --git 
a/regression-test/suites/correctness_p0/test_outer_join_with_inline_view.groovy 
b/regression-test/suites/correctness_p0/test_outer_join_with_inline_view.groovy
index 1a354ab0e3..16f02a39bf 100644
--- 
a/regression-test/suites/correctness_p0/test_outer_join_with_inline_view.groovy
+++ 
b/regression-test/suites/correctness_p0/test_outer_join_with_inline_view.groovy
@@ -69,4 +69,93 @@ suite("test_outer_join_with_inline_view") {
         on a.k1 = b.k1
         order by a.v1; 
     """
+
+    sql """
+        drop table if exists subquery_table_1;
+    """
+
+    sql """
+        drop table if exists subquery_table_2;
+    """
+
+    sql """
+        drop table if exists subquery_table_3;
+    """
+
+    sql """
+        CREATE TABLE `subquery_table_1` (
+        `org_code` varchar(96) NULL 
+        ) ENGINE=OLAP
+        DUPLICATE KEY(`org_code`)
+        COMMENT 'OLAP'
+        DISTRIBUTED BY HASH(`org_code`) BUCKETS 2
+        PROPERTIES (
+        "replication_allocation" = "tag.location.default: 1",
+        "in_memory" = "false",
+        "storage_format" = "V2",
+        "disable_auto_compaction" = "false"
+        );
+    """
+
+    sql """
+        CREATE TABLE `subquery_table_2` (
+        `SKU_CODE` varchar(96) NULL 
+        ) ENGINE=OLAP
+        DUPLICATE KEY(`SKU_CODE`)
+        COMMENT 'OLAP'
+        DISTRIBUTED BY HASH(`SKU_CODE`) BUCKETS 2
+        PROPERTIES (
+        "replication_allocation" = "tag.location.default: 1",
+        "in_memory" = "false",
+        "storage_format" = "V2",
+        "disable_auto_compaction" = "false"
+        );
+    """
+
+    sql """
+        CREATE TABLE `subquery_table_3` (
+        `org_code` varchar(96) NULL, 
+        `bo_ql_in_advance` decimal(27, 6) NULL
+        ) ENGINE=OLAP
+        DUPLICATE KEY(`org_code`)
+        COMMENT 'OLAP'
+        DISTRIBUTED BY HASH(`org_code`) BUCKETS 2
+        PROPERTIES (
+        "replication_allocation" = "tag.location.default: 1",
+        "in_memory" = "false",
+        "storage_format" = "V2",
+        "disable_auto_compaction" = "false"
+        );
+    """
+
+    sql """
+        insert into subquery_table_1 values('1');
+    """
+
+    sql """
+        insert into subquery_table_2 values('1');
+    """
+
+    sql """
+        insert into subquery_table_3 values('1',1);
+    """
+
+    qt_select_with_agg_in_inline_view_and_outer_join """
+        select
+            count(cc.qlnm) as qlnm
+        FROM
+            subquery_table_1 aa
+            left join (
+                SELECT
+                    `s`.`SKU_CODE` AS `org_code`,
+                    coalesce(`t3`.`bo_ql_in_advance`, 0) AS `qlnm`
+                FROM
+                    `subquery_table_2` s
+                inner JOIN 
+                    `subquery_table_3` t3 
+                            ON `s`.`SKU_CODE` = `t3`.`org_code`
+            ) cc on aa.org_code = cc.org_code
+        group by
+            aa.org_code;
+    """
 }


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

Reply via email to