> On Feb. 26, 2015, 10:37 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java, line 459
> > <https://reviews.apache.org/r/31404/diff/1-2/?file=875138#file875138line459>
> >
> >     Not a fan of these timestamp-related changes. It's causing you to have 
> > to create 2 new arrays of Converters which seems awkward.
> >     
> >     I was going to suggest simply using a DateConverter, but I see that the 
> > original behavior also allowed timestamp format strings (really, it allowed 
> > anything after the 'yyyy-mm-dd' part). Rather than have 2 different 
> > converters, maybe just go back to what you had before, use a single 
> > converter to string, and then just apply whatever format logic you are 
> > going to apply to the string value.

Sure I can use SimpleDateFormat("yyyy-MM-dd") to convert short and long strings 
to Date. That was the original behaviour in last_day, next_day, add_months.
methods (obtainDateConvertor and getDateValue)

I also noticed that GenericUDFFromUtcTimestamp uses TimestampConverter for all 
input types. To support this behaviour I created 2 methods 
(obtainTimestampConvertor and getTimestampValue)


> On Feb. 26, 2015, 10:37 p.m., Jason Dere wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFNextDay.java,
> >  line 80
> > <https://reviews.apache.org/r/31404/diff/2/?file=878682#file878682line80>
> >
> >     So this is actually an example of something that used to be valid 
> > input, but no longer works due to the change from SimpleDateFormat to 
> > Timestamp/Date converter. This would be ok for the UDFs you've changed 
> > since they are new ones, but I don't know if this kind of change in 
> > behavior would be something that would be a concern if we were to apply 
> > this same refactoring to some of the other date-related UDFs.

fixed this. all selects in last_day, next_day, add_months q files return what 
they originally returned now


- Alexander


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


On Feb. 26, 2015, 9:16 p.m., Alexander Pivovarov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31404/
> -----------------------------------------------------------
> 
> (Updated Feb. 26, 2015, 9:16 p.m.)
> 
> 
> Review request for hive, Jason Dere and Thejas Nair.
> 
> 
> Bugs: HIVE-9744
>     https://issues.apache.org/jira/browse/HIVE-9744
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9744 Move common arguments validation and value extraction code to 
> GenericUDF
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java 
> 8a0f573648c51c4945be8ffec4a0b06dfa7061c8 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAddMonths.java 
> c5968835a74195bea6b31a5c7b7346907fed5ce0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFInitCap.java 
> 406fcd608a13fadb8902bf273932acb05a0f3bbe 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLastDay.java 
> 3a43c571ae3a83924a00413181a62ce6f4408125 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLevenstein.java 
> de41793ba3925aa9e1ad9623d92881c57791f047 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFNextDay.java 
> 38f08b74609a4018221ca3f5b92cf33799604d60 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFAddMonths.java
>  4ccae97a227257294d69f728426f425d060ef0c7 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFLastDay.java 
> 4b233a6966bbdf6902c53f2aaf53cc0eb422b205 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFLevenshtein.java
>  e674d9f38cf7b5cdffcad6eca07dba74ff1e834b 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFNextDay.java 
> e2ec551d4ae39d521680ee93c791f14f27811270 
>   ql/src/test/queries/clientpositive/udf_next_day.q 
> db821f00c39b84c4bc59ea70b0cc0aacf69cbd18 
>   ql/src/test/results/clientnegative/udf_add_months_error_1.q.out 
> 8226ac6fe89c38fcc14edeea215cd5cce7258683 
>   ql/src/test/results/clientnegative/udf_add_months_error_2.q.out 
> f00949e9a12285cc91032215372975753c1f3b4a 
>   ql/src/test/results/clientnegative/udf_last_day_error_1.q.out 
> 6e718a0c15e84d89b1cfe7f36231e472ff03c37f 
>   ql/src/test/results/clientnegative/udf_last_day_error_2.q.out 
> dc8e3d14f14205ce65355cd53a95cfc788f45fe0 
>   ql/src/test/results/clientnegative/udf_next_day_error_1.q.out 
> c67b9c42f7e7fdf20caa34d028b02fd4819e8343 
>   ql/src/test/results/clientnegative/udf_next_day_error_2.q.out 
> e3cb6a447bf7bd9f09648e46ace1bcba4da55339 
>   ql/src/test/results/clientpositive/udf_add_months.q.out 
> 8c37fc282a25e350435ff6a4b39e02fc2d4f17df 
>   ql/src/test/results/clientpositive/udf_last_day.q.out 
> 2d39e3897625c7651b110779b89652f0d785bc92 
> 
> Diff: https://reviews.apache.org/r/31404/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Pivovarov
> 
>

Reply via email to