OSDir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Review Request 67073: HIVE-19370 : Retain time part in add_months function on timestamp datatype fields in hive



> 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
> 
>