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



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java
<https://reviews.apache.org/r/30697/#comment117064>

    If the main reason to copy ReduceExpressionRule is because the constructor 
is private, then we probably need consider modify Calcite/Optia library, to 
expose the constructor. Copying this class will make it hard to keep this new 
class sync with the any future change/fix made in Calcite/Optiq code.



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java
<https://reviews.apache.org/r/30697/#comment117066>

    One way to solve the issue of missing EMPTYREL: can we let the rule 
generates EMPTYREL first, then use another rule to convert EMPTYREL into LIMIT 
0 (represented in Calcite/Optiq by a SORTREL with fetch=0 and offset=0.)



exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java
<https://reviews.apache.org/r/30697/#comment117065>

    Did we overload isDeterministic() and isDynamicFunction() for 
DrillSqlOperator and its subclass? Otherwise, the default behaviour will 
recognize "random() + 10" as constant, or expression using any UDF which has 
similar properties as constant, which is not right.
    
      public boolean isDeterministic() {
        return true;
      }
    
      public boolean isDynamicFunction() {
        return false;
      }


- Jinfeng Ni


On Feb. 5, 2015, 2: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, 2: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
> 
>

Reply via email to