You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Jason Altekruse <al...@gmail.com> on 2015/02/28 01:15:28 UTC

Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

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

Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.


Bugs: DRILL-2060
    https://issues.apache.org/jira/browse/DRILL-2060


Repository: drill-git


Description
-------

The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.

I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.


Diffs
-----

  exec/interpreter/pom.xml 20539a8 
  exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
  exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
  exec/pom.xml e27e50b 

Diff: https://reviews.apache.org/r/31567/diff/


Testing
-------

Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.


Thanks,

Jason Altekruse


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Jason Altekruse <al...@gmail.com>.

> On March 1, 2015, 4:16 p.m., Aman Sinha wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 237
> > <https://reviews.apache.org/r/31567/diff/1/?file=881347#file881347line237>
> >
> >     Is this meant for expressions such as '2.0 + cast(3+4 as double)' ?  It's probably not very common pattern but you could extract the cast input and minor type and invoke the type-specific visitor.

I have added a test case for this in my updated patch. This substitution is happening earlier in expression materialization so it does not have to happen here.


- Jason


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


On March 5, 2015, 12:25 a.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated March 5, 2015, 12:25 a.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2060
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/#review74701
-----------------------------------------------------------

Ship it!


Refactoring and related changes look ok to me.


exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java
<https://reviews.apache.org/r/31567/#comment121369>

    Is this meant for expressions such as '2.0 + cast(3+4 as double)' ?  It's probably not very common pattern but you could extract the cast input and minor type and invoke the type-specific visitor.


- Aman Sinha


On Feb. 28, 2015, 12:15 a.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2015, 12:15 a.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2060
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Jinfeng Ni <jn...@maprtech.com>.

> On March 4, 2015, 6:55 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 298
> > <https://reviews.apache.org/r/31567/diff/2/?file=885225#file885225line298>
> >
> >     I assume that we are aware that there is performance overhead to use reflection here. So, in case if we want to use interpreter to evaluate many batches, there would be performance impact. On the other hand, we may argue that using interpreter already means performance is not as good as the current run-time compiling + evaluation model, and therefore, adding additional performance overhead does not change a lot.
> 
> Jason Altekruse wrote:
>     While there are no instances of evaluating a full batch of records, much less a series of batches in the current code with the interpreter, I am not too concerned about this. If we need a little more perforamnce we could modify the DrillFuncHolderExpr class to hold hard references to the Fields for the inputs extracted out of the DrillFuncCalsses with reflection once during a setup step. We would still be using some reflection to set the values, but we can remove the overhead of finding the fields by name each time. Overall if we need performance for lots of prepeatitive expression evaluations, we already have a mechanism for it. I will admit we could probably improve the situation for a medium number of evaluations (maybe in the 10,000 range) with these improvements.

Make sense to me.


> On March 4, 2015, 6:55 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 320
> > <https://reviews.apache.org/r/31567/diff/2/?file=885225#file885225line320>
> >
> >     Do you have a plan to move setup() call into places such that setup() will be called once for each VectorAccessible input? 
> >     
> >     In the code compile + evaluation model, doSetup() will be called per batch, in stead of per row.
> 
> Jason Altekruse wrote:
>     I have started working on a fix for this. Its a little complicated with setting constant inputs before the setup method is called. I'm trying to figure out the best way to share code with the rest of the input passing used in the EvalVisitor. Would you be okay with this being opened as an enhancement request to be merged a little later? Considering the current use of the interpreter this won't have an impact on any actual queries today.

Yes, I'm fine that you enhance this later in a new JIRA.


- Jinfeng


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


On March 5, 2015, 5:18 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated March 5, 2015, 5:18 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2060
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Jason Altekruse <al...@gmail.com>.

> On March 5, 2015, 2:55 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java, line 64
> > <https://reviews.apache.org/r/31567/diff/2/?file=885228#file885228line64>
> >
> >     Do we have a testcase which use "like", "similar_to" function?  Those functions are a bit special, in that it requires some input to be constant.

Will add a test case for this.


> On March 5, 2015, 2:55 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 298
> > <https://reviews.apache.org/r/31567/diff/2/?file=885225#file885225line298>
> >
> >     I assume that we are aware that there is performance overhead to use reflection here. So, in case if we want to use interpreter to evaluate many batches, there would be performance impact. On the other hand, we may argue that using interpreter already means performance is not as good as the current run-time compiling + evaluation model, and therefore, adding additional performance overhead does not change a lot.

While there are no instances of evaluating a full batch of records, much less a series of batches in the current code with the interpreter, I am not too concerned about this. If we need a little more perforamnce we could modify the DrillFuncHolderExpr class to hold hard references to the Fields for the inputs extracted out of the DrillFuncCalsses with reflection once during a setup step. We would still be using some reflection to set the values, but we can remove the overhead of finding the fields by name each time. Overall if we need performance for lots of prepeatitive expression evaluations, we already have a mechanism for it. I will admit we could probably improve the situation for a medium number of evaluations (maybe in the 10,000 range) with these improvements.


> On March 5, 2015, 2:55 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 320
> > <https://reviews.apache.org/r/31567/diff/2/?file=885225#file885225line320>
> >
> >     Do you have a plan to move setup() call into places such that setup() will be called once for each VectorAccessible input? 
> >     
> >     In the code compile + evaluation model, doSetup() will be called per batch, in stead of per row.

I have started working on a fix for this. Its a little complicated with setting constant inputs before the setup method is called. I'm trying to figure out the best way to share code with the rest of the input passing used in the EvalVisitor. Would you be okay with this being opened as an enhancement request to be merged a little later? Considering the current use of the interpreter this won't have an impact on any actual queries today.


- Jason


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


On March 5, 2015, 12:25 a.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated March 5, 2015, 12:25 a.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2060
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Jason Altekruse <al...@gmail.com>.

> On March 5, 2015, 2:55 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 320
> > <https://reviews.apache.org/r/31567/diff/2/?file=885225#file885225line320>
> >
> >     Do you have a plan to move setup() call into places such that setup() will be called once for each VectorAccessible input? 
> >     
> >     In the code compile + evaluation model, doSetup() will be called per batch, in stead of per row.
> 
> Jason Altekruse wrote:
>     I have started working on a fix for this. Its a little complicated with setting constant inputs before the setup method is called. I'm trying to figure out the best way to share code with the rest of the input passing used in the EvalVisitor. Would you be okay with this being opened as an enhancement request to be merged a little later? Considering the current use of the interpreter this won't have an impact on any actual queries today.
> 
> Jinfeng Ni wrote:
>     Yes, I'm fine that you enhance this later in a new JIRA.

Made a JIRA to track this, will post my intermediate work there shortly.

https://issues.apache.org/jira/browse/DRILL-2395


- Jason


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


On March 6, 2015, 1:18 a.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated March 6, 2015, 1:18 a.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2060
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/#review75278
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java
<https://reviews.apache.org/r/31567/#comment122251>

    If we pass in Class<T>, then, we could call newInstance() direct.



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java
<https://reviews.apache.org/r/31567/#comment122250>

    Since the new code will use an instance of DrillFunc to do the interpreted expr evaluation, is it better to pass in Class<T>, where T extends DrillFunc, than passing in the interpreted evaluator class name?
    
    In FunctionConverter.getHolder(), we have the reference to the Class<T>. Seems to me it would be good to passing the object of Class<T> when call constructor of DrillSimpleFuncHolder, etc..



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java
<https://reviews.apache.org/r/31567/#comment122253>

    I assume that we are aware that there is performance overhead to use reflection here. So, in case if we want to use interpreter to evaluate many batches, there would be performance impact. On the other hand, we may argue that using interpreter already means performance is not as good as the current run-time compiling + evaluation model, and therefore, adding additional performance overhead does not change a lot.



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java
<https://reviews.apache.org/r/31567/#comment122252>

    Do you have a plan to move setup() call into places such that setup() will be called once for each VectorAccessible input? 
    
    In the code compile + evaluation model, doSetup() will be called per batch, in stead of per row.



exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java
<https://reviews.apache.org/r/31567/#comment122254>

    Do we have a testcase which use "like", "similar_to" function?  Those functions are a bit special, in that it requires some input to be constant.


- Jinfeng Ni


On March 4, 2015, 4:25 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 4:25 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2060
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2406 - part 1 - (previously 2060) refactor interpreted expression evaluation

Posted by Jinfeng Ni <jn...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/#review76812
-----------------------------------------------------------

Ship it!


Ship It!

- Jinfeng Ni


On March 16, 2015, 10:10 a.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated March 16, 2015, 10:10 a.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060 and DRILL-2406
>     https://issues.apache.org/jira/browse/DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2406
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Cluster tests are passing, waiting on unit tests.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2406 - part 1 - (previously 2060) refactor interpreted expression evaluation

Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/#review76802
-----------------------------------------------------------

Ship it!


Ship It!

- Aman Sinha


On March 16, 2015, 5:10 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31567/
> -----------------------------------------------------------
> 
> (Updated March 16, 2015, 5:10 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.
> 
> 
> Bugs: DRILL-2060 and DRILL-2406
>     https://issues.apache.org/jira/browse/DRILL-2060
>     https://issues.apache.org/jira/browse/DRILL-2406
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.
> 
> I have created this to isolate the changes from the others in 2060.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/pom.xml 20539a8 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
>   exec/pom.xml e27e50b 
> 
> Diff: https://reviews.apache.org/r/31567/diff/
> 
> 
> Testing
> -------
> 
> Cluster tests are passing, waiting on unit tests.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 31567: 2406 - part 1 - (previously 2060) refactor interpreted expression evaluation

Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/
-----------------------------------------------------------

(Updated March 16, 2015, 5:10 p.m.)


Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.


Changes
-------

Small changes based on revision to earlier commit ( see https://reviews.apache.org/r/30754/diff/3-4/ for more details)


Bugs: DRILL-2060 and DRILL-2406
    https://issues.apache.org/jira/browse/DRILL-2060
    https://issues.apache.org/jira/browse/DRILL-2406


Repository: drill-git


Description
-------

The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.

I have created this to isolate the changes from the others in 2060.


Diffs (updated)
-----

  exec/interpreter/pom.xml 20539a8 
  exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
  exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
  exec/pom.xml e27e50b 

Diff: https://reviews.apache.org/r/31567/diff/


Testing
-------

Cluster tests are passing, waiting on unit tests.


Thanks,

Jason Altekruse


Re: Review Request 31567: 2406 - part 1 - (previously 2060) refactor interpreted expression evaluation

Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/
-----------------------------------------------------------

(Updated March 9, 2015, 9:53 p.m.)


Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.


Changes
-------

No new code changes, just updating the message as the JIRA number has changed to allow this to be merged indepedently of the constant folding rule.


Summary (updated)
-----------------

2406 - part 1 - (previously 2060) refactor interpreted expression evaluation


Bugs: DRILL-2060 and DRILL-2406
    https://issues.apache.org/jira/browse/DRILL-2060
    https://issues.apache.org/jira/browse/DRILL-2406


Repository: drill-git


Description (updated)
-------

The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.

I have created this to isolate the changes from the others in 2060.


Diffs (updated)
-----

  exec/interpreter/pom.xml 20539a8 
  exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
  exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
  exec/pom.xml e27e50b 

Diff: https://reviews.apache.org/r/31567/diff/


Testing (updated)
-------

Cluster tests are passing, waiting on unit tests.


Thanks,

Jason Altekruse


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/
-----------------------------------------------------------

(Updated March 6, 2015, 1:18 a.m.)


Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.


Changes
-------

Addressed several comments from Aman and Jinfeng, issues not resolved have been left appropriately for further discussion.


Bugs: DRILL-2060
    https://issues.apache.org/jira/browse/DRILL-2060


Repository: drill-git


Description
-------

The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.

I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.


Diffs (updated)
-----

  exec/interpreter/pom.xml 20539a8 
  exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
  exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
  exec/pom.xml e27e50b 

Diff: https://reviews.apache.org/r/31567/diff/


Testing
-------

Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.


Thanks,

Jason Altekruse


Re: Review Request 31567: 2060 - part 1 - refactor interpreted expression evaluation

Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31567/
-----------------------------------------------------------

(Updated March 5, 2015, 12:25 a.m.)


Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, and Mehant Baid.


Changes
-------

Small change to allow this to be built without the pat 2 patch for 2060 which is still being debugged in a few cases.


Bugs: DRILL-2060
    https://issues.apache.org/jira/browse/DRILL-2060


Repository: drill-git


Description
-------

The interpreter was previously not used in normal execution, it was added with unit tests but never hooked up to an execution component. When trying to use it in the new constant folding issues I ran into build issues that are described in detail on the 2060 JIRA.

I have created this to isolate the changes from the others in 2060 for review, but they are intended to be committed together.


Diffs (updated)
-----

  exec/interpreter/pom.xml 20539a8 
  exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java bc631b8 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillBooleanOPHolder.java 743598a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalAddFuncHolder.java 3871cd7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalCastFuncHolder.java db49173 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalDivScaleFuncHolder.java 683a04f 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalMaxScaleFuncHolder.java aa8e2b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalModScaleFuncHolder.java b5e754e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSetScaleFuncHolder.java 47b8507 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalSumScaleFuncHolder.java cb8bfed 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillDecimalZeroScaleFuncHolder.java 674fc87 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java ec284a7 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 0127e6e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillFuncInterpreter.java 3a83542 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/DrillSimpleFuncInterpreter.java e3696f0 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterBuilder.java 3dac818 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java 0fe36cb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterGenerator.java 6cede33 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/ValueHolderHelper.java 2f5bf6a 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java PRE-CREATION 
  exec/java-exec/src/test/resources/functions/interp/test_input.csv PRE-CREATION 
  exec/pom.xml e27e50b 

Diff: https://reviews.apache.org/r/31567/diff/


Testing
-------

Ran the previous interpreter test, with this and part 2 I am still finishing up some last debugging on a few type-specific test cases.


Thanks,

Jason Altekruse