> 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.
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. - Jacques ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30697/#review71480 ----------------------------------------------------------- On Feb. 7, 2015, 1:20 a.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30697/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2015, 1:20 a.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 > ------- > > 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/interpreter/src/test/java/org/apache/drill/exec/expr/TestConstantFolding.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java > 67e1fdb > exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java > c881432 > > 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/DrillRuleSets.java > 3b7adca > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java > PRE-CREATION > > 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 > >
