DRILL-1061: Disable MergeProjectRule if project expression has complex output type
Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/a0a65117 Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/a0a65117 Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/a0a65117 Branch: refs/heads/master Commit: a0a65117534753c79d68d6d0c5d91327631b9dd6 Parents: 3c2402c Author: Mehant Baid <meha...@gmail.com> Authored: Fri Jul 18 11:23:58 2014 -0700 Committer: Aditya Kishore <adi...@maprtech.com> Committed: Thu Jul 24 16:16:02 2014 -0700 ---------------------------------------------------------------------- .../expr/fn/FunctionImplementationRegistry.java | 11 +++ .../planner/logical/DrillMergeProjectRule.java | 72 ++++++++++++++++++++ .../exec/planner/logical/DrillRuleSets.java | 17 +++-- .../drill/exec/planner/sql/DrillSqlWorker.java | 2 +- .../java/org/apache/drill/TestBugFixes.java | 6 ++ 5 files changed, 101 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/a0a65117/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java index d7bc36b..46a24b4 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java @@ -126,4 +126,15 @@ public class FunctionImplementationRegistry { return null; } + + // Method to find if the output type of a drill function if of complex type + public boolean isFunctionComplexOutput(String name) { + List<DrillFuncHolder> methods = drillFuncRegistry.getMethods(name); + for (DrillFuncHolder holder : methods) { + if (holder.getReturnValue().isComplexWriter()) { + return true; + } + } + return false; + } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/a0a65117/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java new file mode 100644 index 0000000..7abc28b --- /dev/null +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java @@ -0,0 +1,72 @@ +/** + * 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.eigenbase.rel.ProjectRelBase; +import org.eigenbase.rel.RelFactories.ProjectFactory; + +import org.eigenbase.rel.rules.MergeProjectRule; +import org.eigenbase.relopt.RelOptRuleCall; + +import org.eigenbase.rex.RexCall; +import org.eigenbase.rex.RexNode; + +import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry; + +public class DrillMergeProjectRule extends MergeProjectRule { + + private FunctionImplementationRegistry functionRegistry; + private static DrillMergeProjectRule INSTANCE = null; + + public static DrillMergeProjectRule getInstance(boolean force, ProjectFactory pFactory, FunctionImplementationRegistry functionRegistry) { + if (INSTANCE == null) { + INSTANCE = new DrillMergeProjectRule(force, pFactory, functionRegistry); + } + return INSTANCE; + } + + private DrillMergeProjectRule(boolean force, ProjectFactory pFactory, FunctionImplementationRegistry functionRegistry) { + super(force, pFactory); + this.functionRegistry = functionRegistry; + } + + @Override + public boolean matches(RelOptRuleCall call) { + ProjectRelBase topProject = call.rel(0); + ProjectRelBase bottomProject = call.rel(1); + + // We have a complex output type do not fire the merge project rule + if (checkComplexOutput(topProject) || checkComplexOutput(bottomProject)) { + return false; + } + + return true; + } + + private boolean checkComplexOutput(ProjectRelBase project) { + for (RexNode expr: project.getChildExps()) { + if (expr instanceof RexCall) { + if (functionRegistry.isFunctionComplexOutput(((RexCall) expr).getOperator().getName())) { + return true; + } + } + } + return false; + } +} http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/a0a65117/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java index d0cd832..63de69c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java @@ -41,7 +41,6 @@ import org.apache.drill.exec.planner.physical.StreamAggPrule; import org.apache.drill.exec.planner.physical.UnionAllPrule; import org.apache.drill.exec.planner.physical.WriterPrule; import org.eigenbase.rel.RelFactories; -import org.eigenbase.rel.rules.MergeProjectRule; import org.eigenbase.rel.rules.PushFilterPastJoinRule; import org.eigenbase.rel.rules.PushFilterPastProjectRule; import org.eigenbase.rel.rules.PushJoinThroughJoinRule; @@ -61,8 +60,12 @@ import com.google.common.collect.ImmutableSet.Builder; public class DrillRuleSets { static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillRuleSets.class); - public static final RuleSet DRILL_BASIC_RULES = new DrillRuleSet(ImmutableSet.of( // - // Add support for WHERE style joins. + public static RuleSet DRILL_BASIC_RULES = null; + + public static RuleSet getDrillBasicRules(QueryContext context) { + if (DRILL_BASIC_RULES == null) { + DRILL_BASIC_RULES = new DrillRuleSet(ImmutableSet.of( // + // Add support for WHERE style joins. PushFilterPastProjectRule.INSTANCE, PushFilterPastJoinRule.FILTER_ON_JOIN, PushFilterPastJoinRule.JOIN, @@ -82,7 +85,7 @@ public class DrillRuleSets { // TableAccessRule.INSTANCE, // //MergeProjectRule.INSTANCE, // - new MergeProjectRule(true, RelFactories.DEFAULT_PROJECT_FACTORY), + DrillMergeProjectRule.getInstance(true, RelFactories.DEFAULT_PROJECT_FACTORY, context.getFunctionRegistry()), RemoveDistinctAggregateRule.INSTANCE, // ReduceAggregatesRule.INSTANCE, // PushProjectPastJoinRule.INSTANCE, @@ -103,9 +106,11 @@ public class DrillRuleSets { DrillLimitRule.INSTANCE, DrillSortRule.INSTANCE, DrillJoinRule.INSTANCE, - DrillUnionRule.INSTANCE, - MergeProjectRule.INSTANCE + DrillUnionRule.INSTANCE )); + } + return DRILL_BASIC_RULES; + } /* public static final RuleSet DRILL_PHYSICAL_MEM = new DrillRuleSet(ImmutableSet.of( // http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/a0a65117/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java index 4cbd674..a48070f 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java @@ -91,7 +91,7 @@ public class DrillSqlWorker { RuleSet drillPhysicalMem = DrillRuleSets.mergedRuleSets( DrillRuleSets.getPhysicalRules(context), storagePluginRegistry.getStoragePluginRuleSet()); - allRules = new RuleSet[] {DrillRuleSets.DRILL_BASIC_RULES, drillPhysicalMem}; + allRules = new RuleSet[] {DrillRuleSets.getDrillBasicRules(context), drillPhysicalMem}; } } } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/a0a65117/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java b/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java index 2aa0618..8348d79 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java @@ -48,4 +48,10 @@ public class TestBugFixes extends BaseTestQuery { public void DRILL883() throws Exception { test("select n1.n_regionkey from cp.`tpch/nation.parquet` n1, (select n_nationkey from cp.`tpch/nation.parquet`) as n2 where n1.n_nationkey = n2.n_nationkey"); } + + @Test + public void DRILL1061() throws Exception { + String query = "select foo.mycol.x as COMPLEX_COL from (select convert_from('{ x : [1,2], y : 100 }', 'JSON') as mycol from cp.`tpch/nation.parquet`) as foo(mycol) limit 1"; + test(query); + } }
