drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Altekruse" <altekruseja...@gmail.com>
Subject Re: Review Request 30697: Drill-2060 - constant expression folding during planning
Date Fri, 06 Feb 2015 23:56:50 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.

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.


- Jason


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30697/#review71480
-----------------------------------------------------------


On Feb. 5, 2015, 10:26 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30697/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 10:26 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
> -------
> 
> 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/io/netty/buffer/FakeAllocator.java 3de0a75 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/Accountor.java 2b48ef0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/AtomicRemainder.java 057cfa6

>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 83d9d1e

>   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/FragmentContext.java dc47f4e

>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67

>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java c881432 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java
412da85 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java d68a5b5

>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java
6db9f4a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java
22fa047 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
d78ba8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java
f09acaa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
52b892e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java
9026661 
>   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 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java
2bb29e5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java
cdb4ba0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/SpoolingRawBatchBuffer.java
6ee93ab 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a

>   exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 92d4b98

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