> On May 16, 2018, 11:32 a.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java > > Lines 85 (patched) > > <https://reviews.apache.org/r/67073/diff/2/?file=2023748#file2023748line85> > > > > I would prefer to thow an exception here instead of working with the > > default version. Better for the user to see the error faster, and not only > > after a long query run, when the output is not formatted as expected. What > > do you think?
Sounds good. I did it that way because I thought we can use default format if user provides invalid value, but it makes sense as the user anyways wants to provide a different format and we can let them know earlier if it is invalid. Removed exception catch so that it will be thrown if format is invalid. > On May 16, 2018, 11:32 a.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java > > Lines 143 (patched) > > <https://reviews.apache.org/r/67073/diff/2/?file=2023748#file2023748line143> > > > > Why is this null check needed? Just added it as it was present in UDFDateFormat. But I think we won't have a null or invalid formatter or null Date in the line String res = formatter.format(newDate); So removing the null check - Bharathkrishna ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67073/#review203208 ----------------------------------------------------------- On May 16, 2018, 5:48 p.m., Bharathkrishna Guruvayoor Murali wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67073/ > ----------------------------------------------------------- > > (Updated May 16, 2018, 5:48 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 > ----- > > 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 > ql/src/test/queries/clientpositive/udf_add_months.q > 0b8eeeeb444fd1657117dec18c2b4e7173767617 > ql/src/test/results/clientpositive/udf_add_months.q.out > 5ba720ae85d30c0da7f94f377d6b324bce850907 > > > Diff: https://reviews.apache.org/r/67073/diff/3/ > > > Testing > ------- > > Added unit tests. > > > Thanks, > > Bharathkrishna Guruvayoor Murali > >
