> On Nov. 19, 2014, 10:46 p.m., Hanifi Gunes wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushFilterPastProjectRule.java, > > line 29 > > <https://reviews.apache.org/r/28207/diff/1/?file=768447#file768447line29> > > > > Afaics we are not preserving item expression on filter. Would there be > > a case where this may break things?
I will check it out. Since this new rule only stops the application of the rule under certain conditions and keeps the behavior of the original rule for other conditions, I did not think it would break anything (since the original rule was not preserving item expr). That said, I saw 1 QA functional test fail with a hash join schema change error and I am trying to see if it caused by this change. > On Nov. 19, 2014, 10:46 p.m., Hanifi Gunes wrote: > > exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java, > > line 211 > > <https://reviews.apache.org/r/28207/diff/1/?file=768450#file768450line211> > > > > Here instead of passing TABLES[X] to test instances we should iterate > > over TABLES and run the same test for each available data format in TABLES > > since pushdown is configurable at data format level. Yes, will do. > On Nov. 19, 2014, 10:46 p.m., Hanifi Gunes wrote: > > exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java, > > line 237 > > <https://reviews.apache.org/r/28207/diff/1/?file=768450#file768450line237> > > > > The same here. will do. - Aman ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28207/#review62254 ----------------------------------------------------------- On Nov. 19, 2014, 2:46 a.m., Aman Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28207/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2014, 2:46 a.m.) > > > Review request for drill and Hanifi Gunes. > > > Repository: drill-git > > > Description > ------- > > DRILL-1707 is caused by a looping in the planner between > PushProjectPastFilter and PushFilterPastProject rules. This started > happening after the DrillPushProjectPastJoin rule was added because with that > rule we are pushing more expressions and that further triggers the other 2 > rules. > In this fix, we don't push Filter past Project if the Project is producing > ITEM expression (this is done as part of a new rule). Currently, the fix does > not check if the Filter conditions are referencing the ITEM expression and > does not split the filter condition into components that can be pushed. That > is marked as a TODO. > > As part of this patch, I enabled a few tests in TestProjectPushDown and > TestExampleQueries that were marked Ignored earlier due to the looping issue. > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushFilterPastProjectRule.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > f4481cb > exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java > bb411e7 > exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java > 6e998a3 > > Diff: https://reviews.apache.org/r/28207/diff/ > > > Testing > ------- > > unit tests in TestProjectPushDown, TestExampleQueries and the failing > mondrian query in DRILL-1707 > > > Thanks, > > Aman Sinha > >
