> On May 11, 2018, 10:08 a.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java > > Lines 74 (patched) > > <https://reviews.apache.org/r/67073/diff/1/?file=2019552#file2019552line74> > > > > 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? > > Bharathkrishna Guruvayoor Murali wrote: > Do you mean that we need to retain the time part also in the case when > input type is String? > My patch would only retain time part if input type is timestamp.
I mean, that GenericUDFMonthsBetween, and GenericUDFDateFormat also handles according the the comments. See: " // the function should support both short date and full timestamp format // time part of the timestamp should not be skipped Date date = getTimestampValue(arguments, 0, tsConverters); if (date == null) { date = getDateValue(arguments, 0, dtInputTypes, dtConverters); if (date == null) { return null; } } " In both cases it uses this approach to first try timestamp, and if that fails trie date. This way it does not have to create separate cases for different primitive types - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67073/#review202907 ----------------------------------------------------------- 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 > >