> On March 27, 2015, 6:08 p.m., Mohit Sabharwal wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java, line 507
> > <https://reviews.apache.org/r/32489/diff/3/?file=907259#file907259line507>
> >
> >     19...a comment on where this comes from would be great
> 
> Alexander Pivovarov wrote:
>     added comment
>           // lets be strict here and
>           // support only exact 10 char string for short date format

getTimestampValue method was added recently as part the activity to add common 
methods to GenericUDF to validate/extract values.
getTimestampValue uses sql.Timestamp.getValue(str) internaly to parse input 
strings.
sql.Timestamp.getValue supports only string which are 19 chars and more.

BTW, getTimestampValue was not used by any UDF before. GenericUDFMonthsBetween 
is the first function which uses it.
I think it's quite common situation when UDF should support both short date 
yyyy-MM-dd and long date+time string formats and not skip time part
>From the other side we need to decid what to do with incorrect strings which 
>contains just hour and minutes for example "2015-03-27 10:30"
original getTimestampValue will return null

The change which I added yesterday - "if (dateStr.length() < 19)" will return 
not null value but time part will be skipped
I think it might be confused for end users. They might expect that if function 
returs not null value then it parses partial time part of the string.

This is why today I decided to return null for such incorrect strings. I think 
it's less confused to end users.

So, I only left support for exact short date format which is 10 chars yyyy-MM-dd


- Alexander


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


On March 27, 2015, 6:35 p.m., Alexander Pivovarov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32489/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 6:35 p.m.)
> 
> 
> Review request for hive and Jason Dere.
> 
> 
> Bugs: HIVE-9518
>     https://issues.apache.org/jira/browse/HIVE-9518
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9518 Implement MONTHS_BETWEEN aligned with Oracle one
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
> 2476e832b8b7101971ea2226368aa82633b7e7d1 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java 
> ce981232382e993c7c9d640efe9b2d21f70a0ed4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMonthsBetween.java
>  PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFMonthsBetween.java
>  PRE-CREATION 
>   ql/src/test/queries/clientpositive/udf_months_between.q PRE-CREATION 
>   ql/src/test/results/clientpositive/show_functions.q.out 
> 22091d06241218a5c0ee21d6ee6be00a71706971 
>   ql/src/test/results/clientpositive/udf_months_between.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/32489/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Pivovarov
> 
>

Reply via email to