> On Feb. 6, 2015, 3:07 a.m., Jinfeng Ni wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java, > > line 72 > > <https://reviews.apache.org/r/30697/diff/1/?file=851496#file851496line72> > > > > 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.
I agree, where is the current Drill fork of optiq hosted? > On Feb. 6, 2015, 3:07 a.m., Jinfeng Ni wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java, > > line 74 > > <https://reviews.apache.org/r/30697/diff/1/?file=851496#file851496line74> > > > > 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.) This seems reasonable, will work on a simple rule to do this. > On Feb. 6, 2015, 3:07 a.m., Jinfeng Ni wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/OptiqForkedConstantReduxRule.java, > > line 628 > > <https://reviews.apache.org/r/30697/diff/1/?file=851496#file851496line628> > > > > 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; > > } Will look into this - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30697/#review71363 ----------------------------------------------------------- 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 > >
