git.net

[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


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