This is an automated email from the ASF dual-hosted git repository. morningman pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/master by this push: new 5c42514 [Bug][SQL]Fix except node child not order correctly (#4003) 5c42514 is described below commit 5c42514a8fb7df364c50506997d0ea8179bd6679 Author: yangzhg <780531...@qq.com> AuthorDate: Tue Jul 7 23:06:36 2020 +0800 [Bug][SQL]Fix except node child not order correctly (#4003) Fixes #3995 ## Why does it happen When SetOperations encounters that the previous node needs Aggregate, the timing of add AggregationNode is wrong. You should add AggregationNode first before add other children. ## Why doesn't intersect and union have this problem intersect and union conform to the commutation law, so it doesn't matter if the order is wrong ## Why this problem has not been tested before In the previous test case, not cover the previous node was not AggregationNode --- .../apache/doris/planner/SingleNodePlanner.java | 35 +++++++++++----------- .../java/org/apache/doris/planner/PlannerTest.java | 10 +++++++ 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java b/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java index f4b502b..a6dd5bf 100644 --- a/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java +++ b/fe/src/main/java/org/apache/doris/planner/SingleNodePlanner.java @@ -1621,6 +1621,23 @@ public class SingleNodePlanner { default: throw new AnalysisException("not supported set operations: " + operation); } + // If it is a union or other same operation, there are only two possibilities, + // one is the root node, and the other is a distinct node in front, so the setOperationDistinctPlan will + // be aggregate node, if this is a mixed operation + // e.g. : + // a union b -> result == null + // a union b union all c -> result == null -> result == AggregationNode + // a union all b except c -> result == null -> result == UnionNode + // a union b except c -> result == null -> result == AggregationNode + if (result != null && result instanceof SetOperationNode) { + Preconditions.checkState(!result.getClass().equals(setOpNode.getClass())); + setOpNode.addChild(result, setOperationStmt.getResultExprs()); + } else if (result != null) { + Preconditions.checkState(setOperationStmt.hasDistinctOps()); + Preconditions.checkState(result instanceof AggregationNode); + setOpNode.addChild(result, + setOperationStmt.getDistinctAggInfo().getGroupingExprs()); + } for (SetOperationStmt.SetOperand op : setOperands) { if (op.getAnalyzer().hasEmptyResultSet()) { unmarkCollectionSlots(op.getQueryStmt()); @@ -1643,23 +1660,6 @@ public class SingleNodePlanner { } setOpNode.addChild(opPlan, op.getQueryStmt().getResultExprs()); } - // If it is a union or other same operation, there are only two possibilities, - // one is the root node, and the other is a distinct node in front, so the setOperationDistinctPlan will - // be aggregate node, if this is a mixed operation - // e.g. : - // a union b -> result == null - // a union b union all c -> result == null -> result == AggregationNode - // a union all b except c -> result == null -> result == UnionNode - // a union b except c -> result == null -> result == AggregationNode - if (result != null && result instanceof SetOperationNode) { - Preconditions.checkState(!result.getClass().equals(setOpNode.getClass())); - setOpNode.addChild(result, setOperationStmt.getResultExprs()); - } else if (result != null) { - Preconditions.checkState(setOperationStmt.hasDistinctOps()); - Preconditions.checkState(result instanceof AggregationNode); - setOpNode.addChild(result, - setOperationStmt.getDistinctAggInfo().getGroupingExprs()); - } setOpNode.init(analyzer); return setOpNode; } @@ -2111,4 +2111,3 @@ public class SingleNodePlanner { return analyzer.getUnassignedConjuncts(tupleIds); } } - diff --git a/fe/src/test/java/org/apache/doris/planner/PlannerTest.java b/fe/src/test/java/org/apache/doris/planner/PlannerTest.java index 19ad093..aac91a4 100644 --- a/fe/src/test/java/org/apache/doris/planner/PlannerTest.java +++ b/fe/src/test/java/org/apache/doris/planner/PlannerTest.java @@ -231,6 +231,16 @@ public class PlannerTest { Assert.assertEquals(2, StringUtils.countMatches(plan9, "UNION")); Assert.assertEquals(3, StringUtils.countMatches(plan9, "INTERSECT")); Assert.assertEquals(2, StringUtils.countMatches(plan9, "EXCEPT")); + + String sql10 = "select 499 union select 670 except select 499"; + StmtExecutor stmtExecutor10 = new StmtExecutor(ctx, sql10); + stmtExecutor10.execute(); + Planner planner10 = stmtExecutor10.planner(); + List<PlanFragment> fragments10 = planner10.getFragments(); + Assert.assertTrue(fragments10.get(0).getPlanRoot().getFragment() + .getPlanRoot().getChild(0) instanceof AggregationNode); + Assert.assertTrue(fragments10.get(0).getPlanRoot() + .getFragment().getPlanRoot().getChild(1) instanceof UnionNode); } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org