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