hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jason Dere" <jd...@hortonworks.com>
Subject Re: Review Request 30437: Implement MONTHS_BETWEEN aligned with Oracle one
Date Fri, 30 Jan 2015 21:44:48 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30437/#review70404
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java
<https://reviews.apache.org/r/30437/#comment115601>

    Rather than create a new IntWritable each time evaluate() is called, can you create the
writable once during initialize() re-use it?



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiffBase.java
<https://reviews.apache.org/r/30437/#comment115577>

    If this initialize() function is shared between DATEDIFF() and MONTHS_BETWEEN(), then
returning INT object inspector is wrong, since MONTHS_BETWEEN can return DOUBLE type. You
will have to return a double type object inspector in that case. Otherwise when Hive tries
to use the results of this UDF there will be invalid cast exceptions because the return value
will be treated as int.



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMonthsBetween.java
<https://reviews.apache.org/r/30437/#comment115602>

    remove trailing whitespace



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMonthsBetween.java
<https://reviews.apache.org/r/30437/#comment115605>

    Create new writable once during initialize()



ql/src/test/TestGenericUDFMonthsBetween.java
<https://reviews.apache.org/r/30437/#comment115586>

    minor complaint, but get rid of these extra whitespace characters



ql/src/test/TestGenericUDFMonthsBetween.java
<https://reviews.apache.org/r/30437/#comment115599>

    Is this calling java.sql.Timestamp.Timestamp(int year, int month, int date, int hour,
int minute, int second, int nano)? Don't use this if so - this is a deprecated API. Use Timestmap.valueOf().
Since that takes a string in 'yyyy-mm-dd hh:mm:ss) format, it also makes it easier to make
sense of the timestamp value.



ql/src/test/TestGenericUDFMonthsBetween.java
<https://reviews.apache.org/r/30437/#comment115600>

    Same here - use Date.valueOf('1995-02-02')


- Jason Dere


On Jan. 30, 2015, 1:43 a.m., XIAOBING ZHOU wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30437/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2015, 1:43 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> This is used to track work to build Oracle like months_between. Here's semantics:
> MONTHS_BETWEEN returns number of months between dates date1 and date2. If date1 is later
than date2, then the result is positive. If date1 is earlier than date2, then the result is
negative. If date1 and date2 are either the same days of the month or both last days of months,
then the result is always an integer. Otherwise Oracle Database calculates the fractional
portion of the result based on a 31-day month and considers the difference in time components
date1 and date2.
> 
> https://issues.apache.org/jira/browse/HIVE-9518
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 23d77ca 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiff.java 1ecd835 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFDateDiffBase.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMonthsBetween.java PRE-CREATION

>   ql/src/test/TestGenericUDFMonthsBetween.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30437/diff/
> 
> 
> Testing
> -------
> 
> SEE ALSO ql/src/test/TestGenericUDFMonthsBetween.java
> 
> 
> Thanks,
> 
> XIAOBING ZHOU
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message