drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacques Nadeau" <jacques.dr...@gmail.com>
Subject Re: Review Request 30697: Drill-2060 - constant expression folding during planning
Date Sun, 08 Feb 2015 02:49:40 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message