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 2020/07/07 15:06:45 UTC
[incubator-doris] branch master updated: [Bug][SQL]Fix except node
child not order correctly (#4003)
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 <78...@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-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org