----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67073/#review202907 -----------------------------------------------------------
Hi Bharath, Thanks for the patch! Few quick questions there (not too familiar with this part of the code yet) Thanks, Peter ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java Lines 74 (patched) <https://reviews.apache.org/r/67073/#comment284958> Do we think this is a better approach then the one used by GenericUDFMonthsBetween, and GenericUDFDateFormat? Are we sure that we know the exact primitive categories we can convert? My guess that we originally accepted string as an input... What happens then? ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java Lines 111 (patched) <https://reviews.apache.org/r/67073/#comment284956> Why do we need this conversion? ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java Lines 130 (patched) <https://reviews.apache.org/r/67073/#comment284961> This code line suggests to me that we accept strings as a first input: checkArgGroups(arguments, 0, inputTypes, STRING_GROUP, DATE_GROUP, VOID_GROUP); What happens when the first input is string? - Peter Vary On May 10, 2018, 9:55 p.m., Bharathkrishna Guruvayoor Murali wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67073/ > ----------------------------------------------------------- > > (Updated May 10, 2018, 9:55 p.m.) > > > Review request for hive, Peter Vary, Sahil Takiar, and Vihang Karajgaonkar. > > > Repository: hive-git > > > Description > ------- > > Adding support to retain the time part (HH:mm:ss) for add_months UDF when the > input is given as timestamp format. > > > Diffs > ----- > > common/src/java/org/apache/hive/common/util/DateUtils.java > 65f3b9401916abdfa52fbf75d115ba6b61758fb0 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java > dae4b97b4a17e98122431e5fda655fd9f873fdb5 > > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java > af9b6c43c7dafc69c4944eab02894786af306f35 > > > Diff: https://reviews.apache.org/r/67073/diff/1/ > > > Testing > ------- > > Added unit tests. > > > Thanks, > > Bharathkrishna Guruvayoor Murali > >
