hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Szehon Ho" <sze...@cloudera.com>
Subject Re: Review Request 39248: Fix Greatest/Least UDF to SQL standard (HIVE-12070 and HIVE-12082)
Date Wed, 14 Oct 2015 01:02:02 GMT

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


Changes
-------

Address review comments.


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 (updated)
-----

  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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message