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