----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15113/#review27928 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseNumeric.java <https://reviews.apache.org/r/15113/#comment54449> This early out is incorrect if it's 2 decimal TypeInfos that are the same. decimal(10,2) * decimal(10,2) != decimal(10,2) ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseNumeric.java <https://reviews.apache.org/r/15113/#comment54450> I think this logic may also be incorrect - decimal(10,2) * ??? would very likely be a result other than decimal(10,2). I think string here should be equivalent to double. ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseNumeric.java <https://reviews.apache.org/r/15113/#comment54354> Should an exception be thrown either here or by the caller? ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPDivide.java <https://reviews.apache.org/r/15113/#comment54452> same as comments about about string type. ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPDivide.java <https://reviews.apache.org/r/15113/#comment54446> I think you are over inspecting here :) convert() can use the left variable directly, no need to inspect the value of left. Playing with this I got an error when I tried to add the 2 columns from a table. So all of these instances of convert() from all of these UDFs needs to be changed. Please also add tests to verify the result TypeInfo of the the arithmetic operations using various different types. - Jason Dere On Oct. 31, 2013, 5:43 a.m., Xuefu Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15113/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2013, 5:43 a.m.) > > > Review request for hive. > > > Bugs: HIVe-5356 > https://issues.apache.org/jira/browse/HIVe-5356 > > > Repository: hive-git > > > Description > ------- > > Replace plus, minus, and so on 6 old UDFs with generic UDF implementations. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 898b6a5 > ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java > 120095a > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFBaseNumericOp.java 1e74fce > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPDivide.java da01934 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPMinus.java 0996231 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPMod.java 0942ac3 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPMultiply.java 07c1957 > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFOPPlus.java cfe7b2b > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFPosMod.java 0da7eae > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseNumeric.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPDivide.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMinus.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMod.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPMultiply.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPPlus.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFPosMod.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorSelectOperator.java > 4aeb4e6 > > ql/src/test/org/apache/hadoop/hive/ql/exec/vector/TestVectorizationContext.java > 90ab983 > ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFOPMod.java 4c2e722 > ql/src/test/org/apache/hadoop/hive/ql/udf/TestUDFPosMod.java 2a301bc > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFOPMod.java > PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFPosMod.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/15113/diff/ > > > Testing > ------- > > Full set of unit tests is to be run. Old testcases are also migrated. > > > Thanks, > > Xuefu Zhang > >
