> On March 16, 2015, 9:22 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRules.java,
> >  line 40
> > <https://reviews.apache.org/r/30697/diff/4/?file=896064#file896064line40>
> >
> >     I notice the rule class name is plural (Rules instead of Rule) - could 
> > you just use 'Rule' such that it is consistent with other rules. I 
> > understand you are creating 2 instances but I think we do that elsewhere 
> > also.

Fair enough, seems to be a pattern in calcite as well.


> On March 16, 2015, 9:22 p.m., Aman Sinha wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/TestConstantFolding.java,
> >  line 58
> > <https://reviews.apache.org/r/30697/diff/4/?file=896069#file896069line58>
> >
> >     You might want to add a test case with slightly more complex expression 
> > containing constants and functions:  e.g  ABS((6-18)/(2*3)) should produce 
> > 2.  Also add a test case with string literals.

Added.


- Jason


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


On March 20, 2015, 5:30 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30697/
> -----------------------------------------------------------
> 
> (Updated March 20, 2015, 5:30 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
> -------
> 
> THIS REQUEST HAS BEEN BROKEN INTO TWO REVIEWS:
> - I had to make a fairly substandial change to the interpreteed expression 
> evaluation to make the tests work.
> 
> 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
> -----
> 
>   
> contrib/storage-hive/core/src/main/java/org/apache/drill/exec/planner/sql/HiveUDFOperator.java
>  7524690 
>   exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java d87fb76 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java
>  32cf362 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctionHelpers.java
>  3b1d7ef 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java
>  35c35ec 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/BufferManager.java 
> 536f6fd 
>   
> 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/DrillReduceExpressionsRule.java
>  PRE-CREATION 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java
>  b1a7189 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
>  f320157 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java
>  710418b 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
>  608fac7 
>   exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java e673230 
>   exec/java-exec/src/test/java/org/apache/drill/TestProjectPushDown.java 
> cbb5c6e 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java
>  72ad31a 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/TestConstantFolding.java
>  PRE-CREATION 
>   exec/java-exec/src/test/resources/parquet/alltypes.json PRE-CREATION 
>   pom.xml 05573a1 
> 
> 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