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 2023/02/26 14:14:27 UTC
[doris] 02/03: [fix](planner) the project expr should be calculated in join node in some case (#17035)
This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-1.2-lts
in repository https://gitbox.apache.org/repos/asf/doris.git
commit dd7cd70bc8b016c008e911eb8d9d97fb9fe0b0d3
Author: starocean999 <40...@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 5f60d15e1d..d9b012c6b4 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
@@ -127,6 +127,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 093b3d61b3..fd861842ce 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
@@ -679,7 +679,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 f1822c7d8a..d21b44c7e5 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);
}
@@ -1575,7 +1575,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.
@@ -1592,7 +1592,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())) {
@@ -1602,7 +1602,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;
}
@@ -1633,7 +1634,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-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org