----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39248/#review102671 -----------------------------------------------------------
Ship it! Ship It! - Xuefu Zhang On Oct. 14, 2015, 1:02 a.m., Szehon Ho wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39248/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2015, 1:02 a.m.) > > > Review request for hive. > > > Bugs: HIVE-12082 > https://issues.apache.org/jira/browse/HIVE-12082 > > > Repository: hive-git > > > Description > ------- > > See HIVE-12070 and HIVE-12082 for discussions and findings. Refactored the > Greatest/Least UDF's to be inline with the SQL-standard spec of comparison > operators, and the mysql implementation of greatest/least functional UDF's. > > The main functional changes is that: > 1. Different types can be compared now, but used to throw an exception. > Comparison uses the same logic as binary comparison operators, ie greaterThan. > 2. If any argument is NULL, the result is null. NULLs used to be ignored in > the comparison in favor of non-null values, which violates the SQL-standard. > > Code changes: > Common logics is captured in the new class 'GenericUDFBaseNWayCompare', which > does a linear comparison in the two-cases where arguments are of same type > and different type, in the latter it uses Converters. The class that it uses > is ObjectInspectorUtils.compare(), which is the same as the binary comparison > operators. > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseNwayCompare.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFGreatest.java > e1eab89 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLeast.java > 64a1b47 > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFGreatest.java > 55d7d5d > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFLeast.java > 47e4801 > ql/src/test/queries/clientnegative/udf_greatest_error_2.q b270a1a > ql/src/test/queries/clientnegative/udf_greatest_error_3.q ba21748 > ql/src/test/queries/clientnegative/udf_greatest_error_4.q ae6d928 > ql/src/test/queries/clientpositive/udf_greatest.q 02c7d3c > ql/src/test/queries/clientpositive/udf_least.q a754ef0 > ql/src/test/results/clientnegative/udf_greatest_error_2.q.out 9a6348c > ql/src/test/results/clientnegative/udf_greatest_error_3.q.out 3fb3499 > ql/src/test/results/clientnegative/udf_greatest_error_4.q.out 58b4c44 > ql/src/test/results/clientpositive/udf_greatest.q.out 10f1c2d > ql/src/test/results/clientpositive/udf_least.q.out 6983137 > > Diff: https://reviews.apache.org/r/39248/diff/ > > > Testing > ------- > > Added more unit tests, and q tests. > > > Thanks, > > Szehon Ho > >
