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