----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16329/#review30678 -----------------------------------------------------------
The patch overall looks fine to. A minor suggestion below. I am wondering if the string value (string, char or varchar) is in the decimal format (scientific or non-scientific), the converting to double won't work. Do you think it's better to convert the string values to decimal in all these cases ? If not, it might be helpful to document that behavior. ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRound.java <https://reviews.apache.org/r/16329/#comment58773> I do agree that "Only numeric data types allowed" is a bit misleading since we do support string types as well .. - Prasad Mujumdar On Dec. 17, 2013, 9:26 p.m., Xuefu Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16329/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2013, 9:26 p.m.) > > > Review request for hive and Prasad Mujumdar. > > > Bugs: HIVE-6039 > https://issues.apache.org/jira/browse/HIVE-6039 > > > Repository: hive-git > > > Description > ------- > > Allow input to these UDFs for char and varchar. > > > Diffs > ----- > > data/files/char_varchar_udf.txt PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFAverage.java > 4b219bd > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFSum.java > 41d5efd > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRound.java > fc9c1b2 > ql/src/test/queries/clientpositive/char_varchar_udf.q PRE-CREATION > ql/src/test/results/clientpositive/char_varchar_udf.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/16329/diff/ > > > Testing > ------- > > Unit tested. New test added. Test suite passed. > > > Thanks, > > Xuefu Zhang > >
