Repository: incubator-drill Updated Branches: refs/heads/master 774a1f04c -> 163917ea1
DRILL-1707: Don't push Filter past Project if the Filter is referencing ITEM expression produced by the Project (this is done as part of a new rule). Enable a few tests in TestProjectPushDown and TestExampleQueries that were marked Ignored earlier. Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/163917ea Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/163917ea Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/163917ea Branch: refs/heads/master Commit: 163917ea11aea3e3252d3f4209cea7d235f4d656 Parents: 774a1f0 Author: Aman Sinha <asi...@maprtech.com> Authored: Tue Nov 18 17:22:07 2014 -0800 Committer: Aman Sinha <asi...@maprtech.com> Committed: Tue Nov 25 12:22:40 2014 -0800 ---------------------------------------------------------------------- .../logical/DrillPushFilterPastProjectRule.java | 120 +++++++++++++++++++ .../exec/planner/logical/DrillRuleSets.java | 55 +-------- .../org/apache/drill/TestExampleQueries.java | 1 - .../org/apache/drill/TestProjectPushDown.java | 47 ++++---- 4 files changed, 143 insertions(+), 80 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/163917ea/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushFilterPastProjectRule.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushFilterPastProjectRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushFilterPastProjectRule.java new file mode 100644 index 0000000..e2310f7 --- /dev/null +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushFilterPastProjectRule.java @@ -0,0 +1,120 @@ +/** + * 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 java.util.List; + +import org.apache.drill.common.expression.FieldReference; +import org.apache.drill.common.expression.SchemaPath; +import org.eigenbase.rel.CalcRel; +import org.eigenbase.rel.FilterRel; +import org.eigenbase.rel.ProjectRel; +import org.eigenbase.relopt.RelOptRule; +import org.eigenbase.relopt.RelOptRuleCall; +import org.eigenbase.relopt.RelOptUtil; +import org.eigenbase.reltype.RelDataTypeField; +import org.eigenbase.rex.RexCall; +import org.eigenbase.rex.RexInputRef; +import org.eigenbase.rex.RexNode; +import org.eigenbase.rex.RexVisitor; +import org.eigenbase.rex.RexVisitorImpl; +import org.eigenbase.sql.SqlOperator; +import org.eigenbase.util.Util; + +public class DrillPushFilterPastProjectRule extends RelOptRule { + + public final static RelOptRule INSTANCE = new DrillPushFilterPastProjectRule(); + + private RexCall findItemOperator( + final RexNode node, + final List<RexNode> projExprs) { + try { + RexVisitor<Void> visitor = + new RexVisitorImpl<Void>(true) { + public Void visitCall(RexCall call) { + if ("item".equals(call.getOperator().getName().toLowerCase())) { + throw new Util.FoundOne(call); /* throw exception to interrupt tree walk (this is similar to + other utility methods in RexUtil.java */ + } + return super.visitCall(call); + } + + public Void visitInputRef(RexInputRef inputRef) { + final int index = inputRef.getIndex(); + RexNode n = projExprs.get(index); + if (n instanceof RexCall) { + RexCall r = (RexCall) n; + if ("item".equals(r.getOperator().getName().toLowerCase())) { + throw new Util.FoundOne(r); + } + } + + return super.visitInputRef(inputRef); + } + }; + node.accept(visitor); + return null; + } catch (Util.FoundOne e) { + Util.swallow(e, null); + return (RexCall) e.getNode(); + } + } + + protected DrillPushFilterPastProjectRule() { + super( + operand( + FilterRel.class, + operand(ProjectRel.class, any()))); + } + + //~ Methods ---------------------------------------------------------------- + + // implement RelOptRule + public void onMatch(RelOptRuleCall call) { + FilterRel filterRel = call.rel(0); + ProjectRel projRel = call.rel(1); + + // Don't push Filter past Project if the Filter is referencing an ITEM expression + // from the Project. + //\TODO: Ideally we should split up the filter conditions into ones that + // reference the ITEM expression and ones that don't and push the latter past the Project + if (findItemOperator(filterRel.getCondition(), projRel.getProjects()) != null) { + return; + } + + // convert the filter to one that references the child of the project + RexNode newCondition = + RelOptUtil.pushFilterPastProject(filterRel.getCondition(), projRel); + + FilterRel newFilterRel = + new FilterRel( + filterRel.getCluster(), + projRel.getChild(), + newCondition); + + + ProjectRel newProjRel = + (ProjectRel) CalcRel.createProject( + newFilterRel, + projRel.getNamedProjects(), + false); + + call.transformTo(newProjRel); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/163917ea/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 f4481cb..3b7adca 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 @@ -66,7 +66,8 @@ public class DrillRuleSets { if (DRILL_BASIC_RULES == null) { DRILL_BASIC_RULES = new DrillRuleSet(ImmutableSet.of( // // Add support for WHERE style joins. - PushFilterPastProjectRule.INSTANCE, +// PushFilterPastProjectRule.INSTANCE, // Replaced by DrillPushFilterPastProjectRule + DrillPushFilterPastProjectRule.INSTANCE, PushFilterPastJoinRule.FILTER_ON_JOIN, PushFilterPastJoinRule.JOIN, PushJoinThroughJoinRule.RIGHT, @@ -123,58 +124,6 @@ public class DrillRuleSets { return DRILL_BASIC_RULES; } - /* - public static final RuleSet DRILL_PHYSICAL_MEM = new DrillRuleSet(ImmutableSet.of( // -// DrillScanRule.INSTANCE, -// DrillFilterRule.INSTANCE, -// DrillProjectRule.INSTANCE, -// DrillAggregateRule.INSTANCE, -// -// DrillLimitRule.INSTANCE, -// DrillSortRule.INSTANCE, -// DrillJoinRule.INSTANCE, -// DrillUnionRule.INSTANCE, - - ConvertCountToDirectScan.AGG_ON_PROJ_ON_SCAN, - ConvertCountToDirectScan.AGG_ON_SCAN, - - SortConvertPrule.INSTANCE, - SortPrule.INSTANCE, - ProjectPrule.INSTANCE, - ScanPrule.INSTANCE, - ScreenPrule.INSTANCE, - ExpandConversionRule.INSTANCE, - StreamAggPrule.INSTANCE, - HashAggPrule.INSTANCE, - MergeJoinPrule.INSTANCE, - HashJoinPrule.INSTANCE, - FilterPrule.INSTANCE, - LimitPrule.INSTANCE, - WindowPrule.INSTANCE, - - WriterPrule.INSTANCE, - PushLimitToTopN.INSTANCE - -// ExpandConversionRule.INSTANCE, -// SwapJoinRule.INSTANCE, -// RemoveDistinctRule.INSTANCE, -// UnionToDistinctRule.INSTANCE, -// RemoveTrivialProjectRule.INSTANCE, -// RemoveTrivialCalcRule.INSTANCE, -// RemoveSortRule.INSTANCE, -// -// TableAccessRule.INSTANCE, // -// MergeProjectRule.INSTANCE, // -// PushFilterPastProjectRule.INSTANCE, // -// PushFilterPastJoinRule.FILTER_ON_JOIN, // -// RemoveDistinctAggregateRule.INSTANCE, // -// ReduceAggregatesRule.INSTANCE, // -// SwapJoinRule.INSTANCE, // -// PushJoinThroughJoinRule.RIGHT, // -// PushJoinThroughJoinRule.LEFT, // -// PushSortPastProjectRule.INSTANCE, // - )); -*/ public static final RuleSet DRILL_PHYSICAL_DISK = new DrillRuleSet(ImmutableSet.of( // ProjectPrule.INSTANCE http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/163917ea/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java index b9d9a27..6c13dba 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java @@ -386,7 +386,6 @@ public class TestExampleQueries extends BaseTestQuery{ } @Test - @Ignore("This test has been ignored until DRILL-1680 is fixed") public void testSelectStartSubQueryJoinWithWhereClause() throws Exception { // select clause, where, on, group by, order by. test(" select n.n_regionkey, count(*) as cnt \n" + http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/163917ea/exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java b/exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java index 6e998a3..cbb5c6e 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java @@ -182,9 +182,9 @@ public class TestProjectPushDown extends PlanTestBase { final String pushDownSqlPattern = "select %s from cp.`%s` t0, cp.`%s` t1, cp.`%s` t2 where %s"; final String projection = "t0.fcolumns[0], t0.fmy.field, t0.freally.nested.field[0], t1.scolumns[0], t1.smy.field, t1.sreally.nested.field[0], t2.tcolumns[0], t2.tmy.field, t2.treally.nested.field[0]"; final String filter = "t0.fname = t1.sname and t1.slastname = t2.tlastname and t0.fcolumns[1] + t1.scolumns[1] + t2.tcolumns[1]=100"; - final String firstExpected = "\"columns\" : [ \"`fname`\", \"`fcolumns`[1]\", \"`fcolumns`[0]\", \"`fmy`.`field`\", \"`freally`.`nested`.`field`[0]\" ],"; - final String secondExpected = "\"columns\" : [ \"`sname`\", \"`slastname`\", \"`scolumns`[1]\", \"`scolumns`[0]\", \"`smy`.`field`\", \"`sreally`.`nested`.`field`[0]\" ],"; - final String thirdExpected = "\"columns\" : [ \"`tlastname`\", \"`tcolumns`[1]\", \"`tcolumns`[0]\", \"`tmy`.`field`\", \"`treally`.`nested`.`field`[0]\" ],"; + final String firstExpected = "\"columns\" : [ \"`fname`\", \"`fcolumns`[0]\", \"`fmy`.`field`\", \"`freally`.`nested`.`field`[0]\", \"`fcolumns`[1]\" ],"; + final String secondExpected = "\"columns\" : [ \"`sname`\", \"`slastname`\", \"`scolumns`[0]\", \"`smy`.`field`\", \"`sreally`.`nested`.`field`[0]\", \"`scolumns`[1]\" ],"; + final String thirdExpected = "\"columns\" : [ \"`tlastname`\", \"`tcolumns`[0]\", \"`tmy`.`field`\", \"`treally`.`nested`.`field`[0]\", \"`tcolumns`[1]\" ],"; for (String table: TABLES) { testPushDown(new PushDownTestInstance(pushDownSqlPattern, @@ -193,43 +193,38 @@ public class TestProjectPushDown extends PlanTestBase { } @Test - @Ignore("This query does not work when ProjectPastJoin is enabled. This is a known issue.") public void testProjectPastJoinPastFilterPastJoinPushDown() throws Exception { final String pushDownSqlPattern = "select %s from cp.`%s` t0, cp.`%s` t1, cp.`%s` t2 where %s"; final String projection = "t0.fcolumns[0], t0.fmy.field, t0.freally.nested.field[0], t1.scolumns[0], t1.smy.field, t1.sreally.nested.field[0], t2.tcolumns[0], t2.tmy.field, t2.treally.nested.field[0]"; final String filter = "t0.fname = t1.sname and t1.slastname = t2.tlastname and t0.fcolumns[1] + t1.scolumns[1] = 100"; final String firstExpected = "\"columns\" : [ \"`fname`\", \"`fcolumns`[1]\", \"`fcolumns`[0]\", \"`fmy`.`field`\", \"`freally`.`nested`.`field`[0]\" ],"; final String secondExpected = "\"columns\" : [ \"`sname`\", \"`slastname`\", \"`scolumns`[1]\", \"`scolumns`[0]\", \"`smy`.`field`\", \"`sreally`.`nested`.`field`[0]\" ],"; - final String thirdExpected = "\"columns\" : [ \"`tlastname`\", \"`tcolumns`[1]\", \"`tcolumns`[0]\", \"`tmy`.`field`\", \"`treally`.`nested`.`field`[0]\" ],"; - - final String[] TABLES = new String[] { - "project/pushdown/empty0.json", - "project/pushdown/empty1.json", - "project/pushdown/empty2.json", - }; -// for (String table: TABLES) { - testPushDown(new PushDownTestInstance(pushDownSqlPattern, - new String[]{firstExpected, secondExpected, thirdExpected}, projection, TABLES[0], TABLES[1], TABLES[2], filter)); -// } + final String thirdExpected = "\"columns\" : [ \"`tlastname`\", \"`tcolumns`[0]\", \"`tmy`.`field`\", \"`treally`.`nested`.`field`[0]\" ],"; + + for (String table: TABLES) { + testPushDown(new PushDownTestInstance(pushDownSqlPattern, + new String[]{firstExpected, secondExpected, thirdExpected}, projection, table, table, table, filter)); + } + } @Test - @Ignore("This query does not work when ProjectPastJoin is enabled. This is a known issue.") public void testSimpleProjectPastJoinPastFilterPastJoinPushDown() throws Exception { - String sql = "select * " + - "from cp.`%s` t0, cp.`%s` t1, cp.`%s` t2 " + - "where t0.fname = t1.sname and t1.slastname = t2.tlastname and t0.fcolumns[0] + t1.scolumns = 100"; +// String sql = "select * " + +// "from cp.`%s` t0, cp.`%s` t1, cp.`%s` t2 " + +// "where t0.fname = t1.sname and t1.slastname = t2.tlastname and t0.fcolumns[0] + t1.scolumns = 100"; - final String[] TABLES = new String[] { - "project/pushdown/empty0.json", - "project/pushdown/empty1.json", - "project/pushdown/empty2.json", - }; + final String firstExpected = "\"columns\" : [ \"`a`\", \"`fa`\", \"`fcolumns`[0]\" ],"; + final String secondExpected = "\"columns\" : [ \"`a`\", \"`b`\", \"`c`\", \"`sa`\" ],"; + final String thirdExpected = "\"columns\" : [ \"`d`\", \"`ta`\" ],"; - sql = "select t0.fa, t1.sa, t2.ta " + + String sql = "select t0.fa, t1.sa, t2.ta " + " from cp.`%s` t0, cp.`%s` t1, cp.`%s` t2 " + " where t0.a=t1.b and t1.c=t2.d and t0.fcolumns[0] + t1.a = 100"; - testPushDown(new PushDownTestInstance(sql, "nothing", TABLES[0],TABLES[1],TABLES[2])); + for (String table: TABLES) { + testPushDown(new PushDownTestInstance(sql, + new String[]{firstExpected, secondExpected, thirdExpected}, table,table,table)); + } } protected void testPushDown(PushDownTestInstance test) throws Exception {
