You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Venki Korukanti <ve...@gmail.com> on 2014/02/21 22:10:12 UTC

Re: Review Request 18372: Hive UDFs in Drill

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

(Updated Feb. 21, 2014, 9:09 p.m.)


Review request for drill, Jacques Nadeau and Jinfeng Ni.


Repository: drill-git


Description
-------

Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.

There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.

github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb


Diffs
-----

  common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
  common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
  common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
  common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
  common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
  common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
  common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
  common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
  common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
  common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
  common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
  common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
  common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
  common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
  common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
  common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
  common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
  common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
  common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
  common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
  common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
  common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
  common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
  common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
  common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
  contrib/pom.xml dc07979 
  contrib/storage-hive/pom.xml PRE-CREATION 
  contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
  exec/java-exec/pom.xml 1c4dc32 
  exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
  exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
  exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
  exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
  exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
  exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
  exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
  exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
  exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
  exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 

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


Testing
-------

GenericUDF.json and UDF.json contain the testcases for both types of UDFs.


Thanks,

Venki Korukanti


Re: Review Request 18372: Hive UDFs in Drill

Posted by Venki Korukanti <ve...@gmail.com>.

> On Feb. 28, 2014, 6:02 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java, line 96
> > <https://reviews.apache.org/r/18372/diff/2/?file=504196#file504196line96>
> >
> >     We should put the following logic in visitFunctionHoldeExpression, otherwise, ConstantExpressionIdentifier is broken.
> >     
> >     checkChildren(holder, value, holder.isAggregating());
> >

Currently there is no method in Holder to identify whether the method is aggregate or not. Added an interface method to DrillFuncHolder and HiveFuncHolder. Also corrected the ConstantExpressionIdentifier as mentioned above.

I see ConstantExpressionIdentifier is not used anywhere except in commented section of EvaluationVisitor.addExpr. As we talked offline, for now I am putting a TODO to revisit this method after isDeterministic flag added to FunctionTemplate.


- Venki


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


On Feb. 26, 2014, 2:20 a.m., Venki Korukanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18372/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2014, 2:20 a.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.
> 
> There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.
> 
> github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb
> 
> 
> Diffs
> -----
> 
>   common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
>   common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
>   common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
>   common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
>   common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
>   common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
>   common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
>   common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
>   common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
>   common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
>   common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
>   common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
>   common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
>   common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
>   common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
>   common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
>   common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
>   contrib/pom.xml dc07979 
>   contrib/storage-hive/pom.xml PRE-CREATION 
>   contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
>   exec/java-exec/pom.xml 1c4dc32 
>   exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
>   exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735 
>   exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
>   exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 
>   sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 9ae9682 
> 
> Diff: https://reviews.apache.org/r/18372/diff/
> 
> 
> Testing
> -------
> 
> GenericUDF.json and UDF.json contain the testcases for both types of UDFs.
> 
> 
> Thanks,
> 
> Venki Korukanti
> 
>


Re: Review Request 18372: Hive UDFs in Drill

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



exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java
<https://reviews.apache.org/r/18372/#comment66501>

    We should put the following logic in visitFunctionHoldeExpression, otherwise, ConstantExpressionIdentifier is broken.
    
    checkChildren(holder, value, holder.isAggregating());
    


- Jinfeng Ni


On Feb. 25, 2014, 6:20 p.m., Venki Korukanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18372/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 6:20 p.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.
> 
> There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.
> 
> github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb
> 
> 
> Diffs
> -----
> 
>   common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
>   common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
>   common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
>   common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
>   common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
>   common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
>   common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
>   common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
>   common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
>   common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
>   common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
>   common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
>   common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
>   common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
>   common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
>   common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
>   common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
>   contrib/pom.xml dc07979 
>   contrib/storage-hive/pom.xml PRE-CREATION 
>   contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
>   exec/java-exec/pom.xml 1c4dc32 
>   exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
>   exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735 
>   exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
>   exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 
>   sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 9ae9682 
> 
> Diff: https://reviews.apache.org/r/18372/diff/
> 
> 
> Testing
> -------
> 
> GenericUDF.json and UDF.json contain the testcases for both types of UDFs.
> 
> 
> Thanks,
> 
> Venki Korukanti
> 
>


Re: Review Request 18372: Hive UDFs in Drill

Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18372/#review35288
-----------------------------------------------------------



common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java
<https://reviews.apache.org/r/18372/#comment65738>

    You should caps to name static variables



common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java
<https://reviews.apache.org/r/18372/#comment65740>

    Let's discuss.  Aggregate checking is important.



exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java
<https://reviews.apache.org/r/18372/#comment65741>

    rather than using a string delimiter, let's use an array of strings.


- Jacques Nadeau


On Feb. 28, 2014, 9:35 p.m., Venki Korukanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18372/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2014, 9:35 p.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.
> 
> There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.
> 
> github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb
> 
> 
> Diffs
> -----
> 
>   common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
>   common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
>   common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
>   common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
>   common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
>   common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
>   common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
>   common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
>   common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
>   common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
>   common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
>   common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
>   common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
>   common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
>   common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
>   common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
>   common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
>   contrib/pom.xml dc07979 
>   contrib/storage-hive/pom.xml PRE-CREATION 
>   contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
>   exec/java-exec/pom.xml 1c4dc32 
>   exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
>   exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java 20c0746 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735 
>   exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
>   exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 
>   sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 9ae9682 
> 
> Diff: https://reviews.apache.org/r/18372/diff/
> 
> 
> Testing
> -------
> 
> GenericUDF.json and UDF.json contain the testcases for both types of UDFs.
> 
> 
> Thanks,
> 
> Venki Korukanti
> 
>


Re: Review Request 18372: Hive UDFs in Drill

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



common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java
<https://reviews.apache.org/r/18372/#comment70295>

    On line 44, for AggregateChecker and ConstantChecker, when there is nested aggregated function call, you will throw IllegalArgumentException. This is different from the logic in method visitIfExpression, where error message is added into a ErrorCollector. Do you think we had better to make the behavior consistent in ExpressionValidator? If yes, we need pass in ErrorCollector to AggregateChecker.
    
    Similar for ConstantChecker on line 47. 



common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
<https://reviews.apache.org/r/18372/#comment70297>

    I feel the logic here is not right. You only check whether the i-th argument is constant when argConstantOnly(i) is true. If not, then you did not check the argument at all, but you will still return "true". 
    
    For example,   1 + col seems to return true from this method. 
    


- Jinfeng Ni


On March 20, 2014, 10:04 a.m., Venki Korukanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18372/
> -----------------------------------------------------------
> 
> (Updated March 20, 2014, 10:04 a.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.
> 
> There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.
> 
> github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb
> 
> 
> Diffs
> -----
> 
>   common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
>   common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
>   common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
>   common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 076c150 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java f9e408b 
>   common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
>   common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 7cb386a 
>   common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
>   common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
>   common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
>   common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
>   common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 4be3820 
>   common/src/main/java/org/apache/drill/common/expression/fn/CastFunctions.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java e1d3390 
>   common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
>   common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
>   common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 469cedd 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 36e6a52 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java a951a60 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java 87d146a 
>   common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 71266f7 
>   common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
>   contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
>   exec/java-exec/src/main/codegen/config.fmpp fcf1f4d 
>   exec/java-exec/src/main/codegen/data/HiveTypes.tdd PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/CastFunctions.java af32a27 
>   exec/java-exec/src/main/codegen/templates/CastFunctionsSrcVarLen.java 5855381 
>   exec/java-exec/src/main/codegen/templates/CastFunctionsSrcVarLenTargetVarLen.java 8594d5a 
>   exec/java-exec/src/main/codegen/templates/CastFunctionsTargetVarLen.java c864e72 
>   exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
>   exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java 9ec2178 
>   exec/java-exec/src/main/codegen/templates/MathFunctions.java c683261 
>   exec/java-exec/src/main/codegen/templates/ObjectInspectorHelper.java PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/ObjectInspectors.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 83a5e6f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java cac47b3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java c0f34f4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java 2208e53 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 5f1e358 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java d1619b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java de847df 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 3145240 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/agg/impl/CountFunctions.java b0939f1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/agg/impl/SumFunctions.java 57905fe 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java f65af3b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java 0b43028 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 3cc82b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a1d3368 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 3fcf5c9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java 9212867 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java 36cfb53 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java ad60e8f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6e98543 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 8572085 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 3413046 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 3364a45 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRel.java b4c1bf9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java 13f5494 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillParseContext.java b82fef5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java 0fdff3c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0e6b7bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java baecc3f 
>   exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java 7ec7243 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestSimpleFunctions.java 5bfe369 
>   exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
>   exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18372/diff/
> 
> 
> Testing
> -------
> 
> GenericUDF.json and UDF.json contain the testcases for both types of UDFs.
> 
> 
> Thanks,
> 
> Venki Korukanti
> 
>


Re: Review Request 18372: Hive UDFs in Drill

Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18372/
-----------------------------------------------------------

(Updated March 20, 2014, 5:04 p.m.)


Review request for drill, Jacques Nadeau and Jinfeng Ni.


Changes
-------

Rebased the patch to latest master.
Converter ObjectInspectors and ObjectInspectorHelper into freemarker/CodeModel codegen.


Repository: drill-git


Description
-------

Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.

There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.

github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb


Diffs (updated)
-----

  common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
  common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
  common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
  common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
  common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
  common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 076c150 
  common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
  common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java f9e408b 
  common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
  common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 7cb386a 
  common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
  common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
  common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
  common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
  common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 4be3820 
  common/src/main/java/org/apache/drill/common/expression/fn/CastFunctions.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java e1d3390 
  common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
  common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
  common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
  common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 469cedd 
  common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 36e6a52 
  common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java a951a60 
  common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java 87d146a 
  common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 71266f7 
  common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
  contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
  exec/java-exec/src/main/codegen/config.fmpp fcf1f4d 
  exec/java-exec/src/main/codegen/data/HiveTypes.tdd PRE-CREATION 
  exec/java-exec/src/main/codegen/templates/CastFunctions.java af32a27 
  exec/java-exec/src/main/codegen/templates/CastFunctionsSrcVarLen.java 5855381 
  exec/java-exec/src/main/codegen/templates/CastFunctionsSrcVarLenTargetVarLen.java 8594d5a 
  exec/java-exec/src/main/codegen/templates/CastFunctionsTargetVarLen.java c864e72 
  exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
  exec/java-exec/src/main/codegen/templates/MathFunctionTemplates.java 9ec2178 
  exec/java-exec/src/main/codegen/templates/MathFunctions.java c683261 
  exec/java-exec/src/main/codegen/templates/ObjectInspectorHelper.java PRE-CREATION 
  exec/java-exec/src/main/codegen/templates/ObjectInspectors.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 83a5e6f 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java cac47b3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java c0f34f4 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/FunctionTemplate.java 2208e53 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 5f1e358 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java d1619b2 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java de847df 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java 3145240 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/agg/impl/CountFunctions.java b0939f1 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/agg/impl/SumFunctions.java 57905fe 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java f65af3b 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java 0b43028 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 3cc82b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a1d3368 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java 3fcf5c9 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java 9212867 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java 36cfb53 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java ad60e8f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 6e98543 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 8572085 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 3413046 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 3364a45 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillAggregateRel.java b4c1bf9 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java 13f5494 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillParseContext.java b82fef5 
  exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java 0fdff3c 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0e6b7bf 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java baecc3f 
  exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java 7ec7243 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestSimpleFunctions.java 5bfe369 
  exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
  exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
  exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 

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


Testing
-------

GenericUDF.json and UDF.json contain the testcases for both types of UDFs.


Thanks,

Venki Korukanti


Re: Review Request 18372: Hive UDFs in Drill

Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18372/
-----------------------------------------------------------

(Updated Feb. 28, 2014, 9:35 p.m.)


Review request for drill, Jacques Nadeau and Jinfeng Ni.


Changes
-------

addressed review comments


Repository: drill-git


Description
-------

Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.

There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.

github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb


Diffs (updated)
-----

  common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
  common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
  common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
  common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
  common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
  common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
  common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
  common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
  common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
  common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
  common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
  common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
  common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
  common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
  common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
  common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
  common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
  common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
  common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
  common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
  common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
  common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
  common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
  common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
  common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
  contrib/pom.xml dc07979 
  contrib/storage-hive/pom.xml PRE-CREATION 
  contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
  exec/java-exec/pom.xml 1c4dc32 
  exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
  exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/annotations/Param.java 20c0746 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735 
  exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
  exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
  exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
  exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
  exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
  exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
  exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
  exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 
  sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 9ae9682 

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


Testing
-------

GenericUDF.json and UDF.json contain the testcases for both types of UDFs.


Thanks,

Venki Korukanti


Re: Review Request 18372: Hive UDFs in Drill

Posted by Venki Korukanti <ve...@gmail.com>.

> On Feb. 28, 2014, 2:33 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java, line 330
> > <https://reviews.apache.org/r/18372/diff/2/?file=504198#file504198line330>
> >
> >     ConstantFilter should not see FunctionCall, since it has been converted in materialization step. Here, better to raise exception.

done


> On Feb. 28, 2014, 2:33 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java, line 133
> > <https://reviews.apache.org/r/18372/diff/2/?file=504199#file504199line133>
> >
> >     The error message for failure of cast function resolve is not same as the regular functions (line 149 - 168). 
> >     
> >     It might be better to wrap Line 149-168 in a method, and call this method for both cast function and regular function, so that we have similar style of error message.

done


> On Feb. 28, 2014, 2:33 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java, line 101
> > <https://reviews.apache.org/r/18372/diff/2/?file=504203#file504203line101>
> >
> >     this is probably a minor issue. If the function has a long list of registerNames, then we will output every one in the list?

Probably printing the first name should be sufficient. Changed accordingly.


> On Feb. 28, 2014, 2:33 a.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java, line 1
> > <https://reviews.apache.org/r/18372/diff/2/?file=504222#file504222line1>
> >
> >     Probably it's better to use freemarker to generate the ObjectInspector class, since they look similar for int, bigint, float4, etc....
> >

I am going to log a separate JIRA to:
1. generate ObjectInspectors using freemarker
2. Remove ObjectInspectorHelper and generate the code using codemodel as part of function implementation code generation.


> On Feb. 28, 2014, 2:33 a.m., Jinfeng Ni wrote:
> > common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java, line 40
> > <https://reviews.apache.org/r/18372/diff/2/?file=504169#file504169line40>
> >
> >     For FunctionHolderExpression, we probably should check if an aggregate function is nested within another aggregate function. Similar logic as AggregateChecker. 
> >     
> >     i.e sum(col1 + avg(col2)) should be blocked. 
> >

Added code to:
1. validate nested aggregate functions within aggregate function using AggregateChecker. 
2. validate argument expressions for function arguments that expect constant expressions using ConstantChecker. 

Modified AggregateChecker and ConstantChecker to handle FunctionHolderExpression


> On Feb. 28, 2014, 2:33 a.m., Jinfeng Ni wrote:
> > common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java, line 16
> > <https://reviews.apache.org/r/18372/diff/2/?file=504187#file504187line16>
> >
> >     Looks like ConstantChecker is only used by ref.  Probably we could delete it?  (ConstantExpressionIdentifier, which will be used in EvalVisitor, would still be needed).

ConstantChecker was used in BasicArgumentValidator as part of ExpressionValidator. As BasicArgumentValidator is removed, we lost the check where we verified the function arguments are constants when FunctionDefinition expects constant arguments. So we need ConstantChecker and used as part of ExpressionValidator.


- Venki


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


On Feb. 26, 2014, 2:20 a.m., Venki Korukanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18372/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2014, 2:20 a.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.
> 
> There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.
> 
> github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb
> 
> 
> Diffs
> -----
> 
>   common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
>   common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
>   common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
>   common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
>   common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
>   common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
>   common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
>   common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
>   common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
>   common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
>   common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
>   common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
>   common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
>   common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
>   common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
>   common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
>   common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
>   contrib/pom.xml dc07979 
>   contrib/storage-hive/pom.xml PRE-CREATION 
>   contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
>   exec/java-exec/pom.xml 1c4dc32 
>   exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
>   exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735 
>   exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
>   exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 
>   sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 9ae9682 
> 
> Diff: https://reviews.apache.org/r/18372/diff/
> 
> 
> Testing
> -------
> 
> GenericUDF.json and UDF.json contain the testcases for both types of UDFs.
> 
> 
> Thanks,
> 
> Venki Korukanti
> 
>


Re: Review Request 18372: Hive UDFs in Drill

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



common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java
<https://reviews.apache.org/r/18372/#comment66467>

    For FunctionHolderExpression, we probably should check if an aggregate function is nested within another aggregate function. Similar logic as AggregateChecker. 
    
    i.e sum(col1 + avg(col2)) should be blocked. 
    



common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
<https://reviews.apache.org/r/18372/#comment66474>

    Looks like ConstantChecker is only used by ref.  Probably we could delete it?  (ConstantExpressionIdentifier, which will be used in EvalVisitor, would still be needed). 



exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java
<https://reviews.apache.org/r/18372/#comment66481>

    ConstantFilter should not see FunctionCall, since it has been converted in materialization step. Here, better to raise exception. 



exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java
<https://reviews.apache.org/r/18372/#comment66468>

    The error message for failure of cast function resolve is not same as the regular functions (line 149 - 168). 
    
    It might be better to wrap Line 149-168 in a method, and call this method for both cast function and regular function, so that we have similar style of error message. 



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

    this is probably a minor issue. If the function has a long list of registerNames, then we will output every one in the list? 



exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java
<https://reviews.apache.org/r/18372/#comment66486>

    Probably it's better to use freemarker to generate the ObjectInspector class, since they look similar for int, bigint, float4, etc....
    


- Jinfeng Ni


On Feb. 25, 2014, 6:20 p.m., Venki Korukanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18372/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 6:20 p.m.)
> 
> 
> Review request for drill, Jacques Nadeau and Jinfeng Ni.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.
> 
> There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.
> 
> github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb
> 
> 
> Diffs
> -----
> 
>   common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
>   common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
>   common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
>   common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
>   common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
>   common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
>   common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
>   common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
>   common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
>   common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
>   common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
>   common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
>   common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
>   common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
>   common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
>   common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
>   common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
>   common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
>   common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
>   common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
>   common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
>   common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
>   common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
>   contrib/pom.xml dc07979 
>   contrib/storage-hive/pom.xml PRE-CREATION 
>   contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
>   exec/java-exec/pom.xml 1c4dc32 
>   exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
>   exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735 
>   exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
>   exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
>   exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
>   exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
>   exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
>   exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 
>   sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 9ae9682 
> 
> Diff: https://reviews.apache.org/r/18372/diff/
> 
> 
> Testing
> -------
> 
> GenericUDF.json and UDF.json contain the testcases for both types of UDFs.
> 
> 
> Thanks,
> 
> Venki Korukanti
> 
>


Re: Review Request 18372: Hive UDFs in Drill

Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18372/
-----------------------------------------------------------

(Updated Feb. 26, 2014, 2:20 a.m.)


Review request for drill, Jacques Nadeau and Jinfeng Ni.


Changes
-------

Fixed failures in existing tests. Few tests in JdbcTest are disabled because they are failing in "ref" module which is going to be deleted soon. Will revisit these tests once "ref" module is deleted from master.


Repository: drill-git


Description
-------

Currently works with simple UDFs that implement GenericUDF and UDF interfaces. Not all column types are supported currently. Supported types are BIT, all INTs, FLOAT4/8, VARCHAR/VAR16CHAR. This patch also removes the FunctionDefnitions (and related code) and uses FunctionHolder for function resolution.

There are few tests failing. Currently debugging those. Will update the patch once the failures are resolved.

github branch: https://github.com/vkorukanti/incubator-drill/commits/func-rb


Diffs (updated)
-----

  common/src/main/antlr3/org/apache/drill/common/expression/parser/ExprParser.g b5cf292 
  common/src/main/java/org/apache/drill/common/expression/Arg.java 0dd47ff 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidator.java 18456b5 
  common/src/main/java/org/apache/drill/common/expression/ArgumentValidators.java da46645 
  common/src/main/java/org/apache/drill/common/expression/BasicArgumentValidator.java f6c94d9 
  common/src/main/java/org/apache/drill/common/expression/CallProvider.java 2339261 
  common/src/main/java/org/apache/drill/common/expression/ExpressionStringBuilder.java 69dd9b3 
  common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java 11d97e8 
  common/src/main/java/org/apache/drill/common/expression/FunctionCall.java e3ed4c5 
  common/src/main/java/org/apache/drill/common/expression/FunctionCallFactory.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionDefinition.java 8009632 
  common/src/main/java/org/apache/drill/common/expression/FunctionHolderExpression.java PRE-CREATION 
  common/src/main/java/org/apache/drill/common/expression/FunctionInstance.java cda49e9 
  common/src/main/java/org/apache/drill/common/expression/FunctionRegistry.java 8ffc07a 
  common/src/main/java/org/apache/drill/common/expression/LogicalExpression.java 70c2617 
  common/src/main/java/org/apache/drill/common/expression/NoArgValidator.java eaabfeb 
  common/src/main/java/org/apache/drill/common/expression/fn/AggregationFunctions.java 5b46b78 
  common/src/main/java/org/apache/drill/common/expression/fn/BooleanFunctions.java f21b750 
  common/src/main/java/org/apache/drill/common/expression/fn/CastFunctionDefs.java 6a98f94 
  common/src/main/java/org/apache/drill/common/expression/fn/MathFunctions.java ee3a099 
  common/src/main/java/org/apache/drill/common/expression/fn/StringFunctions.java 1158115 
  common/src/main/java/org/apache/drill/common/expression/fn/TypeFunctions.java 23ca87c 
  common/src/main/java/org/apache/drill/common/expression/fn/UnaryFunctions.java 0a678fd 
  common/src/main/java/org/apache/drill/common/expression/visitors/AbstractExprVisitor.java 711bdb8 
  common/src/main/java/org/apache/drill/common/expression/visitors/AggregateChecker.java 128f284 
  common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java 02ce231 
  common/src/main/java/org/apache/drill/common/expression/visitors/ExprVisitor.java db34ab7 
  common/src/main/java/org/apache/drill/common/expression/visitors/SimpleExprVisitor.java 65a939b 
  common/src/test/java/org/apache/drill/common/expression/parser/TreeTest.java 6833b69 
  contrib/pom.xml dc07979 
  contrib/storage-hive/pom.xml PRE-CREATION 
  contrib/storage-hive/src/main/resources/drill-module.conf PRE-CREATION 
  exec/java-exec/pom.xml 1c4dc32 
  exec/java-exec/src/main/codegen/templates/ComparisonFunctions.java ba9da07 
  exec/java-exec/src/main/java/org/apache/drill/exec/compile/sig/ConstantExpressionIdentifier.java 46dbbe9 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/DrillFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java a7895d3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java 36433ad 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/HiveFuncHolderExpr.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/ImplicitCastBuilder.java 99c26f3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java 7622865 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java dcdf1ea 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillSimpleFuncHolder.java 4939063 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java befa9bf 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java 93f1992 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFuncHolder.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionImplementationRegistry.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/Alternator.java 5bff29e 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/BitFunctions.java 41bb7c6 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ByteSubstring.java 964e4af 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSubstring.java f991a41 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparatorFunctions.java cd0eb21 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/ComparisonFunctionsNullable.java 9be620a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/HashFunctions.java 76683f3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNotNull.java 5ee5ccb 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/IsNull.java a042a5a 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/MathFunctions.java ea251c3 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/SimpleRepeatedFunctions.java 2f838b1 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/AbstractPrimitiveObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBigIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillBitObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillDeferredObject.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat4ObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillFloat8ObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillSmallIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillTinyIntObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVar16CharObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/DrillVarCharObjectInspector.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/hive/ObjectInspectorHelper.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/AggBatch.java 86fea4e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 298b031 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 43d3b45 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java da8978f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 4d04735 
  exec/java-exec/src/test/java/org/apache/drill/exec/expr/ExpressionTest.java 0de64b4 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/GeneratorFunctions.java b79ccd0 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestHiveUDFs.java PRE-CREATION 
  exec/java-exec/src/test/java/org/apache/drill/exec/record/ExpressionTreeMaterializerTest.java b3b7536 
  exec/java-exec/src/test/resources/functions/hive/GenericUDF.json PRE-CREATION 
  exec/java-exec/src/test/resources/functions/hive/UDF.json PRE-CREATION 
  exec/ref/src/main/java/org/apache/drill/exec/ref/eval/SimpleEvaluationVisitor.java 1b04880 
  exec/ref/src/main/java/org/apache/drill/exec/ref/eval/fn/FunctionArguments.java 47c0405 
  exec/ref/src/test/java/org/apache/drill/exec/ref/rops/CollapsingAggregateTest.java d63c0cc 
  exec/ref/src/test/java/org/apache/drill/exec/ref/rops/OrderROPTest.java d4ea473 
  sqlparser/src/test/java/org/apache/drill/jdbc/test/JdbcTest.java 9ae9682 

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


Testing
-------

GenericUDF.json and UDF.json contain the testcases for both types of UDFs.


Thanks,

Venki Korukanti