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/03/05 00:52:25 UTC

Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

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

(Updated March 4, 2015, 11:52 p.m.)


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


Changes
-------

REmoved record batch from one new UDF setup method. Removed useless loop now that functionality is happening through reflection elsewhere. Added UdfUtilties interface to FragmentContext so this can be build without the DRILL-2060 patches. Removed unneeded test.


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


Repository: drill-git


Description
-------

This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.


Diffs (updated)
-----

  exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
  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/impl/DateTypeFunctions.java cc4be89 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
  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/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/expr/holders/ValueHolder.java 5c2adc6 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 

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


Testing (updated)
-------

Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.


Thanks,

Jason Altekruse


Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

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

Ship it!


Ship It!

- Jinfeng Ni


On March 16, 2015, 10:07 a.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30754/
> -----------------------------------------------------------
> 
> (Updated March 16, 2015, 10:07 a.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, Mehant Baid, and Parth Chandra.
> 
> 
> Bugs: DRILL-2143
>     https://issues.apache.org/jira/browse/DRILL-2143
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
>   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/impl/DateTypeFunctions.java cc4be89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
>   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/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/expr/holders/ValueHolder.java 5c2adc6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 108f5bb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30754/diff/
> 
> 
> Testing
> -------
> 
> Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

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

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


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


Changes
-------

Address jinfengs comments, fix a few docs


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


Repository: drill-git


Description
-------

This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.


Diffs (updated)
-----

  exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
  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/impl/DateTypeFunctions.java cc4be89 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
  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/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/expr/holders/ValueHolder.java 5c2adc6 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 108f5bb 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 

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


Testing
-------

Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.


Thanks,

Jason Altekruse


Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

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

> On March 7, 2015, 2:32 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java, line 140
> > <https://reviews.apache.org/r/30754/diff/3/?file=885028#file885028line140>
> >
> >     I guess for now it's fine to add code just to handle Current_Date() function etc. However, in the future, if we have more functions which depends on QueryContext, like Current_version(), Current_Node_Number, etc, are we going to modify again?
> >     
> >     I feel it would be better to add new annotation in the function template for the inject workspace,
> >     
> >     @Inject(get="getQueryDateTimeInfor") QueryDateTimeInfo datetime;
> >     
> >     QueryDateTimeInfo or any new class will extend QueryContextInfo.
> >     
> >     Then, here, we do not need to add one additional "else if" block each time when we add one individual new function which depends on QueryContext.
> 
> Jason Altekruse wrote:
>     I do agree that we should standardize where this is defined, but I don't believe adding it as an argument in the annotation is the best place. This would require any consumers of this "interface" to give the right value despite the fact that it is constant. I think we could simply create a static map that holds a relationship between Injectable classes and their corresponding getter methods on the UDfUtilities interface.

fixed


> On March 7, 2015, 2:32 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 55
> > <https://reviews.apache.org/r/30754/diff/3/?file=885033#file885033line55>
> >
> >     for constantExpr evaluation, why do we need to passin a outVV? Is it better that this method return a ValueHolder directly?
> 
> Jason Altekruse wrote:
>     I agree, I'll make this change as I go to update the second half of 2060, this is the patch that is actually using this new interface.

fixed


- Jason


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


On March 4, 2015, 11:52 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30754/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 11:52 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, Mehant Baid, and Parth Chandra.
> 
> 
> Bugs: DRILL-2143
>     https://issues.apache.org/jira/browse/DRILL-2143
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
>   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/impl/DateTypeFunctions.java cc4be89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
>   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/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/expr/holders/ValueHolder.java 5c2adc6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30754/diff/
> 
> 
> Testing
> -------
> 
> Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

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

> On March 7, 2015, 2:32 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java, line 140
> > <https://reviews.apache.org/r/30754/diff/3/?file=885028#file885028line140>
> >
> >     I guess for now it's fine to add code just to handle Current_Date() function etc. However, in the future, if we have more functions which depends on QueryContext, like Current_version(), Current_Node_Number, etc, are we going to modify again?
> >     
> >     I feel it would be better to add new annotation in the function template for the inject workspace,
> >     
> >     @Inject(get="getQueryDateTimeInfor") QueryDateTimeInfo datetime;
> >     
> >     QueryDateTimeInfo or any new class will extend QueryContextInfo.
> >     
> >     Then, here, we do not need to add one additional "else if" block each time when we add one individual new function which depends on QueryContext.

I do agree that we should standardize where this is defined, but I don't believe adding it as an argument in the annotation is the best place. This would require any consumers of this "interface" to give the right value despite the fact that it is constant. I think we could simply create a static map that holds a relationship between Injectable classes and their corresponding getter methods on the UDfUtilities interface.


> On March 7, 2015, 2:32 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java, line 55
> > <https://reviews.apache.org/r/30754/diff/3/?file=885033#file885033line55>
> >
> >     for constantExpr evaluation, why do we need to passin a outVV? Is it better that this method return a ValueHolder directly?

I agree, I'll make this change as I go to update the second half of 2060, this is the patch that is actually using this new interface.


- Jason


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


On March 4, 2015, 11:52 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30754/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 11:52 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, Mehant Baid, and Parth Chandra.
> 
> 
> Bugs: DRILL-2143
>     https://issues.apache.org/jira/browse/DRILL-2143
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
>   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/impl/DateTypeFunctions.java cc4be89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
>   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/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/expr/holders/ValueHolder.java 5c2adc6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30754/diff/
> 
> 
> Testing
> -------
> 
> Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

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

Ship it!


Have two minor comments. Overall looks good to me.


exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java
<https://reviews.apache.org/r/30754/#comment122807>

    I guess for now it's fine to add code just to handle Current_Date() function etc. However, in the future, if we have more functions which depends on QueryContext, like Current_version(), Current_Node_Number, etc, are we going to modify again?
    
    I feel it would be better to add new annotation in the function template for the inject workspace,
    
    @Inject(get="getQueryDateTimeInfor") QueryDateTimeInfo datetime;
    
    QueryDateTimeInfo or any new class will extend QueryContextInfo.
    
    Then, here, we do not need to add one additional "else if" block each time when we add one individual new function which depends on QueryContext.



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

    for constantExpr evaluation, why do we need to passin a outVV? Is it better that this method return a ValueHolder directly?


- Jinfeng Ni


On March 4, 2015, 3:52 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30754/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 3:52 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, Mehant Baid, and Parth Chandra.
> 
> 
> Bugs: DRILL-2143
>     https://issues.apache.org/jira/browse/DRILL-2143
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
>   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/impl/DateTypeFunctions.java cc4be89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
>   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/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/expr/holders/ValueHolder.java 5c2adc6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30754/diff/
> 
> 
> Testing
> -------
> 
> Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

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

> On March 6, 2015, 9:48 p.m., Mehant Baid wrote:
> > changes look fine to me. Jinfeng could you review the changes to the class InterpreterEvaluator

Mehant noticed one issue in the InterpreterEvaluator already, where we unneccesarily allocate a new buffer for each literal on each evaluation on one record. I have added a new comment to the JIRA I created to track optimizations once we decide we need to improve performance of interpreting more than a few evaluations at a time. https://issues.apache.org/jira/browse/DRILL-2395?focusedCommentId=14350992&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14350992


- Jason


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


On March 4, 2015, 11:52 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30754/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 11:52 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, Mehant Baid, and Parth Chandra.
> 
> 
> Bugs: DRILL-2143
>     https://issues.apache.org/jira/browse/DRILL-2143
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
>   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/impl/DateTypeFunctions.java cc4be89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
>   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/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/expr/holders/ValueHolder.java 5c2adc6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30754/diff/
> 
> 
> Testing
> -------
> 
> Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>


Re: Review Request 30754: DRILL-2143 - part 2 - remove record batch interface from UDF interface

Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30754/#review75551
-----------------------------------------------------------


changes look fine to me. Jinfeng could you review the changes to the class InterpreterEvaluator

- Mehant Baid


On March 4, 2015, 11:52 p.m., Jason Altekruse wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30754/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 11:52 p.m.)
> 
> 
> Review request for drill, Aman Sinha, Jacques Nadeau, Jinfeng Ni, Mehant Baid, and Parth Chandra.
> 
> 
> Bugs: DRILL-2143
>     https://issues.apache.org/jira/browse/DRILL-2143
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This patch fixes the breakage of removing the record batch from the setup method in the DrillFunc interface. It adds an injectable type to bring back the date functions and make the interpreted expression evaluation work with the new interface.
> 
> 
> Diffs
> -----
> 
>   exec/interpreter/src/test/java/org/apache/drill/exec/expr/ExpressionInterpreterTest.java a94ef94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java 279c428 
>   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/impl/DateTypeFunctions.java cc4be89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java a3bc1de 
>   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/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/expr/holders/ValueHolder.java 5c2adc6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/package-info.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryDateTimeInfo.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30754/diff/
> 
> 
> Testing
> -------
> 
> Almost all cluster tests are passing, recieved some failures that seem unrelated and unlikely cased by the changes, but are not reported as expected failures currently. Still need to run full unit tests again with these most recent changes.
> 
> 
> Thanks,
> 
> Jason Altekruse
> 
>