----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31404/#review74226 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java <https://reviews.apache.org/r/31404/#comment120755> Why go away from using the formatter as it was doing before? Are there any ramifications here to switching to Timestamp.valueOf()? I think Timestamp.valueOf() is probably less forgiving when checking the format. ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java <https://reviews.apache.org/r/31404/#comment120750> I'm not sure if all of this is actally necesary, at least at the time that you are trying to retrieve a value from a converter. If you created a Converter with PrimitiveObjectInspectorFactory.writableLongObjectInspector specified as the output type, LongConverter.convert() already has a similar switch statement that will do the necessary conversion to give you back a LongWritable object. So with a properly created Converter, you would just need to call (LongWritable) converter.convert(arguments[i].get()) It would probably be a good idea to still do this checking when creating the Converter object. ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java <https://reviews.apache.org/r/31404/#comment120748> Does this need to check for null values, or does the caller check for them before calling this method? ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java <https://reviews.apache.org/r/31404/#comment120753> Rather than having to convert from long to int (and risk overflow), just accept byte/short/int here, and leave the caller of the UDF with the option to explicitly cast their long parameter to int if they really want it to work with the UDF. ql/src/test/results/clientpositive/udf_last_day.q.out <https://reviews.apache.org/r/31404/#comment120756> What's the cause of the output difference here? - Jason Dere On Feb. 25, 2015, 3:57 a.m., Alexander Pivovarov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31404/ > ----------------------------------------------------------- > > (Updated Feb. 25, 2015, 3:57 a.m.) > > > Review request for hive, Jason Dere and Thejas Nair. > > > Bugs: HIVE-9744 > https://issues.apache.org/jira/browse/HIVE-9744 > > > Repository: hive-git > > > Description > ------- > > HIVE-9744 Move common arguments validation and value extraction code to > GenericUDF > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java > 8a0f573648c51c4945be8ffec4a0b06dfa7061c8 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java > c5968835a74195bea6b31a5c7b7346907fed5ce0 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFInitCap.java > 406fcd608a13fadb8902bf273932acb05a0f3bbe > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLastDay.java > 3a43c571ae3a83924a00413181a62ce6f4408125 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLevenstein.java > de41793ba3925aa9e1ad9623d92881c57791f047 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFNextDay.java > 38f08b74609a4018221ca3f5b92cf33799604d60 > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java > 4ccae97a227257294d69f728426f425d060ef0c7 > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFLevenshtein.java > e674d9f38cf7b5cdffcad6eca07dba74ff1e834b > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFNextDay.java > e2ec551d4ae39d521680ee93c791f14f27811270 > ql/src/test/queries/clientpositive/udf_next_day.q > db821f00c39b84c4bc59ea70b0cc0aacf69cbd18 > ql/src/test/results/clientnegative/udf_add_months_error_1.q.out > 8226ac6fe89c38fcc14edeea215cd5cce7258683 > ql/src/test/results/clientnegative/udf_add_months_error_2.q.out > f00949e9a12285cc91032215372975753c1f3b4a > ql/src/test/results/clientnegative/udf_last_day_error_1.q.out > 6e718a0c15e84d89b1cfe7f36231e472ff03c37f > ql/src/test/results/clientnegative/udf_last_day_error_2.q.out > dc8e3d14f14205ce65355cd53a95cfc788f45fe0 > ql/src/test/results/clientnegative/udf_next_day_error_1.q.out > c67b9c42f7e7fdf20caa34d028b02fd4819e8343 > ql/src/test/results/clientnegative/udf_next_day_error_2.q.out > e3cb6a447bf7bd9f09648e46ace1bcba4da55339 > ql/src/test/results/clientpositive/udf_add_months.q.out > 8c37fc282a25e350435ff6a4b39e02fc2d4f17df > ql/src/test/results/clientpositive/udf_last_day.q.out > 2d39e3897625c7651b110779b89652f0d785bc92 > ql/src/test/results/clientpositive/udf_next_day.q.out > 37f5640fde267f1635d72f23e76fa89de6403ab5 > > Diff: https://reviews.apache.org/r/31404/diff/ > > > Testing > ------- > > > Thanks, > > Alexander Pivovarov > >
