Repository: drill Updated Branches: refs/heads/master c6549e588 -> 24193b1b0
DRILL-3855: Enable FilterSetOpTransposeRule, DrillProjectSetOpTransposeRule closes #1226 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/6f6229a9 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/6f6229a9 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/6f6229a9 Branch: refs/heads/master Commit: 6f6229a9a71c3e6ff3e873809d9ca591633c4129 Parents: c6549e5 Author: Vitalii Diravka <[email protected]> Authored: Thu Apr 19 14:31:17 2018 +0300 Committer: Vitalii Diravka <[email protected]> Committed: Fri Apr 27 16:47:26 2018 +0300 ---------------------------------------------------------------------- .../apache/drill/exec/planner/PlannerPhase.java | 29 ++++- .../apache/drill/exec/planner/RuleInstance.java | 5 + .../logical/DrillProjectSetOpTransposeRule.java | 30 ----- .../planner/sql/handlers/DefaultSqlHandler.java | 3 +- .../java/org/apache/drill/TestUnionAll.java | 116 +++++++++++++------ ...tyResultAfterProjectPushDownOverUnionAll.tsv | 20 ++++ 6 files changed, 131 insertions(+), 72 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/6f6229a9/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java index 96cfa8a..7e4c8c6 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/PlannerPhase.java @@ -178,6 +178,12 @@ public enum PlannerPhase { PlannerPhase.getPhysicalRules(context), getStorageRules(context, plugins, this)); } + }, + + PRE_LOGICAL_PLANNING("Planning with Hep planner only for rules, which are failed for Volcano planner") { + public RuleSet getRules(OptimizerRulesContext context, Collection<StoragePlugin> plugins) { + return PlannerPhase.getSetOpTransposeRules(); + } }; public final String description; @@ -258,8 +264,8 @@ public enum PlannerPhase { Filter push-down related rules */ DrillPushFilterPastProjectRule.INSTANCE, - // Due to infinite loop in planning (DRILL-3257), temporarily disable this rule - //FilterSetOpTransposeRule.INSTANCE, + // Due to infinite loop in planning (DRILL-3257/CALCITE-1271), temporarily use this rule in Hep planner + // RuleInstance.FILTER_SET_OP_TRANSPOSE_RULE, DrillFilterAggregateTransposeRule.INSTANCE, RuleInstance.FILTER_MERGE_RULE, @@ -276,8 +282,8 @@ public enum PlannerPhase { DrillPushProjectPastFilterRule.INSTANCE, DrillPushProjectPastJoinRule.INSTANCE, - // Due to infinite loop in planning (DRILL-3257), temporarily disable this rule - //DrillProjectSetOpTransposeRule.INSTANCE, + // Due to infinite loop in planning (DRILL-3257/CALCITE-1271), temporarily use this rule in Hep planner + // RuleInstance.PROJECT_SET_OP_TRANSPOSE_RULE, RuleInstance.PROJECT_WINDOW_TRANSPOSE_RULE, DrillPushProjectIntoScanRule.INSTANCE, @@ -491,4 +497,19 @@ public enum PlannerPhase { } + /** + * Get an immutable list of rules to transpose SetOp(Union) operator with other operators.<p> + * Note: Used by Hep planner only (failed for Volcano planner - CALCITE-1271) + * + * @return SetOp(Union) transpose rules + **/ + private static RuleSet getSetOpTransposeRules() { + return RuleSets.ofList(ImmutableSet.<RelOptRule>builder() + .add( + RuleInstance.FILTER_SET_OP_TRANSPOSE_RULE, + RuleInstance.PROJECT_SET_OP_TRANSPOSE_RULE + ).build()); + } + + } http://git-wip-us.apache.org/repos/asf/drill/blob/6f6229a9/exec/java-exec/src/main/java/org/apache/drill/exec/planner/RuleInstance.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/RuleInstance.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/RuleInstance.java index 592aae0..f8a394c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/RuleInstance.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/RuleInstance.java @@ -32,11 +32,13 @@ import org.apache.calcite.rel.rules.FilterSetOpTransposeRule; import org.apache.calcite.rel.rules.JoinPushExpressionsRule; import org.apache.calcite.rel.rules.JoinPushThroughJoinRule; import org.apache.calcite.rel.rules.ProjectRemoveRule; +import org.apache.calcite.rel.rules.ProjectSetOpTransposeRule; import org.apache.calcite.rel.rules.ProjectToWindowRule; import org.apache.calcite.rel.rules.ProjectWindowTransposeRule; import org.apache.calcite.rel.rules.ReduceExpressionsRule; import org.apache.calcite.rel.rules.SortRemoveRule; import org.apache.calcite.rel.rules.UnionToDistinctRule; +import org.apache.drill.exec.planner.logical.DrillConditions; import org.apache.drill.exec.planner.logical.DrillRelFactories; /** @@ -89,6 +91,9 @@ public interface RuleInstance { FilterSetOpTransposeRule FILTER_SET_OP_TRANSPOSE_RULE = new FilterSetOpTransposeRule(DrillRelFactories.LOGICAL_BUILDER); + ProjectSetOpTransposeRule PROJECT_SET_OP_TRANSPOSE_RULE = + new ProjectSetOpTransposeRule(DrillConditions.PRESERVE_ITEM, DrillRelFactories.LOGICAL_BUILDER); + ProjectRemoveRule PROJECT_REMOVE_RULE = new ProjectRemoveRule(DrillRelFactories.LOGICAL_BUILDER); http://git-wip-us.apache.org/repos/asf/drill/blob/6f6229a9/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectSetOpTransposeRule.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectSetOpTransposeRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectSetOpTransposeRule.java deleted file mode 100644 index 91c6dc0..0000000 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectSetOpTransposeRule.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.drill.exec.planner.logical; - -import org.apache.calcite.plan.RelOptRule; -import org.apache.calcite.rel.rules.ProjectSetOpTransposeRule; -import org.apache.calcite.rel.rules.PushProjector; - -public class DrillProjectSetOpTransposeRule extends ProjectSetOpTransposeRule { - public final static RelOptRule INSTANCE = new DrillProjectSetOpTransposeRule(DrillConditions.PRESERVE_ITEM); - - protected DrillProjectSetOpTransposeRule(PushProjector.ExprCondition preserveExprCondition) { - super(preserveExprCondition, DrillRelFactories.LOGICAL_BUILDER); - } -} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/drill/blob/6f6229a9/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java index b9fe9ff..1867b46 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java @@ -235,8 +235,9 @@ public class DefaultSqlHandler extends AbstractSqlHandler { try { final RelNode convertedRelNode; + final RelNode setOpTransposeNode = transform(PlannerType.HEP, PlannerPhase.PRE_LOGICAL_PLANNING, relNode); // HEP Directory pruning . - final RelNode pruned = transform(PlannerType.HEP_BOTTOM_UP, PlannerPhase.DIRECTORY_PRUNING, relNode); + final RelNode pruned = transform(PlannerType.HEP_BOTTOM_UP, PlannerPhase.DIRECTORY_PRUNING, setOpTransposeNode); final RelTraitSet logicalTraits = pruned.getTraitSet().plus(DrillRel.DRILL_LOGICAL); if (!context.getPlannerSettings().isHepOptEnabled()) { http://git-wip-us.apache.org/repos/asf/drill/blob/6f6229a9/exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java b/exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java index bdb8b43..38f1d3a 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestUnionAll.java @@ -634,10 +634,14 @@ public class TestUnionAll extends BaseTestQuery { + "order by n_regionkey"; // Validate the plan - final String[] expectedPlan = {".*Filter.*\n" + - ".*UnionAll.*\n" + - ".*Scan.*columns=\\[`n_regionkey`\\].*\n" + - ".*Scan.*columns=\\[`r_regionkey`\\].*"}; + final String[] expectedPlan = {"Sort.*\n" + + ".*UnionAll.*\n" + + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`n_regionkey`\\].*\n" + + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`r_regionkey`\\].*"}; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -665,16 +669,20 @@ public class TestUnionAll extends BaseTestQuery { "where n_nationkey in (1, 2)"; // Validate the plan - final String[] expectedPlan = {"Filter.*\n" + + final String[] expectedPlan = {"Project.*\n" + ".*UnionAll.*\n" + ".*Project.*\n" + ".*HashJoin.*\n" + - ".*Scan.*columns=\\[`n_regionkey`, `n_nationkey`\\].*\n" + - ".*Scan.*columns=\\[`r_regionkey`\\].*\n" + + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`n_regionkey`, `n_nationkey`\\].*\n" + + ".*Scan.*columns=\\[`r_regionkey`\\].*\n" + ".*Project.*\n" + ".*HashJoin.*\n" + - ".*Scan.*columns=\\[`n_regionkey`, `n_nationkey`\\].*\n" + - ".*Scan.*columns=\\[`r_regionkey`\\].*"}; + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`n_regionkey`, `n_nationkey`\\].*\n" + + ".*Scan.*columns=\\[`r_regionkey`\\].*"}; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -702,13 +710,17 @@ public class TestUnionAll extends BaseTestQuery { "where ct < 100", root, root); // Validate the plan - final String[] expectedPlan = {"Filter.*\n" + + final String[] expectedPlan = {"Project.*\n" + ".*UnionAll.*\n" + - ".*StreamAgg.*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`columns`\\[0\\]\\].*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`columns`\\[0\\]\\].*"}; + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*StreamAgg.*\n" + + ".*Project.*\n" + + ".*Scan.*columns=\\[`columns`\\[0\\]\\].*\n" + + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Project.*\n" + + ".*Scan.*columns=\\[`columns`\\[0\\]\\].*"}; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -734,9 +746,9 @@ public class TestUnionAll extends BaseTestQuery { final String[] expectedPlan = {"Project\\(n_nationkey=\\[\\$0\\], n_name=\\[\\$1\\]\\).*\n" + ".*UnionAll.*\n" + ".*Project.*\n" + - ".*Scan.*columns=\\[`n_nationkey`, `n_name`, `n_comment`\\].*\n" + + ".*Scan.*columns=\\[`n_nationkey`, `n_name`\\].*\n" + ".*Project.*\n" + - ".*Scan.*columns=\\[`r_regionkey`, `r_name`, `r_comment`\\].*" + ".*Scan.*columns=\\[`r_regionkey`, `r_name`\\].*" }; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -760,10 +772,8 @@ public class TestUnionAll extends BaseTestQuery { // Validate the plan final String[] expectedPlan = {"Project\\(n_nationkey=\\[\\$0\\]\\).*\n" + ".*UnionAll.*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`n_nationkey`, `n_name`, `n_comment`\\].*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`r_regionkey`, `r_name`, `r_comment`\\].*"}; + ".*Scan.*columns=\\[`n_nationkey`\\].*\n" + + ".*Scan.*columns=\\[`r_regionkey`\\].*"}; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -786,10 +796,10 @@ public class TestUnionAll extends BaseTestQuery { // Validate the plan final String[] expectedPlan = {"UnionAll.*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`n_nationkey`, `n_name`, `n_comment`\\].*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`r_regionkey`, `r_name`, `r_comment`\\].*"}; + ".*Project\\(col=\\[\\*\\(2, \\$0\\)\\]\\).*\n" + + ".*Scan.*columns=\\[`n_nationkey`\\].*\n" + + ".*Project\\(col=\\[\\*\\(2, \\$0\\)\\]\\).*\n" + + ".*Scan.*columns=\\[`r_regionkey`\\].*"}; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -813,11 +823,11 @@ public class TestUnionAll extends BaseTestQuery { "order by col limit 10", root); // Validate the plan - final String[] expectedPlan = {"UnionAll.*\n." + - ".*Project.*\n" + - ".*Scan.*columns=\\[`n_nationkey`, `n_name`, `n_comment`\\].*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`columns`\\[0\\], `columns`\\[1\\], `columns`\\[2\\]\\]"}; + final String[] expectedPlan = {"UnionAll.*\n" + + ".*Project\\(col=\\[\\*\\(2, \\$0\\)\\]\\).*\n" + + ".*Scan.*columns=\\[`n_nationkey`\\].*\n" + + ".*Project\\(col=\\[\\*\\(2, ITEM\\(\\$0, 0\\)\\)\\]\\).*\n" + + ".*Scan.*columns=\\[`columns`\\[0\\]\\]"}; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -841,9 +851,9 @@ public class TestUnionAll extends BaseTestQuery { // Validate the plan final String[] expectedPlan = {"UnionAll.*\n." + "*Project.*\n" + - ".*Scan.*columns=\\[`n_nationkey`, `n_name`, `n_comment`\\].*\n" + + ".*Scan.*columns=\\[`n_comment`, `n_nationkey`, `n_name`\\].*\n" + ".*Project.*\n" + - ".*Scan.*columns=\\[`r_regionkey`, `r_name`, `r_comment`\\]"}; + ".*Scan.*columns=\\[`r_comment`, `r_regionkey`, `r_name`\\]"}; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -866,12 +876,15 @@ public class TestUnionAll extends BaseTestQuery { "where n_nationkey > 0 and n_nationkey < 4"; // Validate the plan - final String[] expectedPlan = {"Filter.*\n" + + final String[] expectedPlan = {"Project.*\n" + ".*UnionAll.*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`n_nationkey`, `n_name`, `n_comment`\\].*\n" + - ".*Project.*\n" + - ".*Scan.*columns=\\[`r_regionkey`, `r_name`, `r_comment`\\]"}; + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`n_nationkey`\\].*\n" + + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`r_regionkey`\\]" + }; final String[] excludedPlan = {}; PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, excludedPlan); @@ -1275,4 +1288,33 @@ public class TestUnionAll extends BaseTestQuery { .run(); } + @Test // DRILL-3855 + public void testEmptyResultAfterProjectPushDownOverUnionAll() throws Exception { + String query = "select n_nationkey from " + + "(select n_nationkey, n_name, n_comment from cp.`tpch/nation.parquet` " + + "union all select r_regionkey, r_name, r_comment from cp.`tpch/region.parquet`) " + + "where n_nationkey > 4"; + + // Validate the plan + final String[] expectedPlan = {"Project.*\n" + + ".*UnionAll.*\n" + + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`n_nationkey`\\].*\n" + + ".*SelectionVectorRemover.*\n" + + ".*Filter.*\n" + + ".*Scan.*columns=\\[`r_regionkey`\\]"}; + + PlanTestBase.testPlanMatchingPatterns(query, expectedPlan, null); + + testBuilder() + .sqlQuery(query) + .unOrdered() + .csvBaselineFile("testframework/testUnionAllQueries/testEmptyResultAfterProjectPushDownOverUnionAll.tsv") + .baselineTypes(TypeProtos.MinorType.INT) + .baselineColumns("n_nationkey") + .build() + .run(); + } + } http://git-wip-us.apache.org/repos/asf/drill/blob/6f6229a9/exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testEmptyResultAfterProjectPushDownOverUnionAll.tsv ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testEmptyResultAfterProjectPushDownOverUnionAll.tsv b/exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testEmptyResultAfterProjectPushDownOverUnionAll.tsv new file mode 100644 index 0000000..4410de3 --- /dev/null +++ b/exec/java-exec/src/test/resources/testframework/testUnionAllQueries/testEmptyResultAfterProjectPushDownOverUnionAll.tsv @@ -0,0 +1,20 @@ +5 +6 +7 +8 +9 +10 +11 +12 +13 +14 +15 +16 +17 +18 +19 +20 +21 +22 +23 +24 \ No newline at end of file
