> On Feb. 6, 2015, 7:11 p.m., Julian Hyde wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java, > > line 135 > > <https://reviews.apache.org/r/30697/diff/1/?file=851496#file851496line135> > > > > The right thing to do -- which is what the Calcite rule does now -- is > > to create a Values relational expression with 0 rows. (We have phased out > > EmptyRel, because an empty Values is equivalent.) Then there are other > > rules that recognize an empty Values and remove more of the plan. > > > > IIRC, Drill doesn't implement Values. But now you're piling technical > > debt on technical debt. Implement Values already!!! Implement as a > > DrillTableScan reading from a constant in-memory JSON file or something. > > Runtime performance doesn't matter. > > > > There are new constant reduction rules in the new Calcite. E.g. > > JoinPushTransitivePredicatesRule. And the old rules now use inferred > > predicates to do more constant reduction. So be sure to use those when you > > upgrade Calcite. > > Jason Altekruse wrote: > Completely agree on Drill needing to just implement Values. It > complicates testing and leaves a gap in the language. I do however believe > this issue might not be completely solved with Values operator, as we need to > return schema even in the case of a false filter. As Drill doesn't know > schema up front, we would have to rewrite these expressions as a limit 0. > > Jacques Nadeau wrote: > We actually need this to behave conditionally. In the cases where all > the incoming columns are known due to only fixed schema columns feeding the > rel, values is the right approach. However, when it is not known, we need > schema propagation so we should convert to limit 0.
For now I have implemented this to just use the limit 0 in all cases. I have made a small modification to calcite to allow these rules to be extended an for subclasses to define custom behavior in this case. I will post a patch to calcite once I have all of the Drill changes finalized based on the calcite patch. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30697/#review71480 ----------------------------------------------------------- On March 16, 2015, 5:17 p.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30697/ > ----------------------------------------------------------- > > (Updated March 16, 2015, 5:17 p.m.) > > > Review request for drill, Aditya Kishore, Aman Sinha, Jacques Nadeau, Jinfeng > Ni, Mehant Baid, and Parth Chandra. > > > Bugs: DRILL-2060 > https://issues.apache.org/jira/browse/DRILL-2060 > > > Repository: drill-git > > > Description > ------- > > THIS REQUEST HAS BEEN BROKEN INTO TWO REVIEWS: > - I had to make a fairly substandial change to the interpreteed expression > evaluation to make the tests work. > > Use a small modification of a rule in optiq and hook up the interpreted > expression evaluator to fold constant expressions (including those with Drill > UDFs) into literals. Fragment memory limits have been disabled, a more > complete refactoring of memory management is planned, the changes made here > were to allow the creation of a childAllocator without access to a fragment > context. I have added two comments next to the modifiations I made to the > optiq rule for our use case. > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java > 00aaec6 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java > 0fe36cb > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillConstExecutor.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRules.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > 496bc9a > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java > 710418b > exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java e673230 > exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java > cbb5c6e > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/TestConstantFolding.java > PRE-CREATION > exec/java-exec/src/test/resources/parquet/alltypes.json PRE-CREATION > pom.xml 0e56ed6 > > Diff: https://reviews.apache.org/r/30697/diff/ > > > Testing > ------- > > This is a work in progress, some testing has been done, no full unit test run > yet > > > Thanks, > > Jason Altekruse > >
