----------------------------------------------------------- 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 > >
