[ 
https://issues.apache.org/jira/browse/PIG-1310?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12849434#action_12849434
 ] 

Alan Gates commented on PIG-1310:
---------------------------------

Since Pig plans to support SQL soon and since many Pig Latin users are familiar 
with SQL, we'd like to pick a datetime format that will work well with SQL.  
While ISO 8601 is not the same as SQL's datetime format, it looks to me like 
translation between the two will be reasonably easy.

DateMonthToISO lacks javadoc comments, making it hard to know what it does or 
how to use it.

The comments in front of other functions like ISOToUnix should be turned into 
javadoc (just change /* to /** and add an introductory sentence) so users can 
read them without needing to open the code itself.

It might be helpful in your javadocs to provide links to jodatime and somewhere 
that gives a good intro to ISO8601 date formats so users can figure out things 
like:  "What does that Z at the end of the datetime string mean?"

In UnixToISO your example shows an input of long, but the code assumes it's a 
string and parses the string into a long.  It should probably be the former, 
but whichever way you decide to do it the code and the comments should match.

In CustomFormatToISO the comments only show one input (the datetime string), 
but the code assumes two inputs, the datetime string and the format.  The 
comments should reflect this as well as give users an indication of how to 
construct the format string in a way that jodatime will understand it (or 
perhaps just link to somewhere in jodatime that it explains this).

All throughout, if there is an error in parsing the date the code depends on 
jodatime to throw an exception with a meaningful error message.  Have you 
tested that these error messages are reasonably helpful to users?  For now, in 
piggybank, this is ok.  If these eventually move into Pig proper these errors 
will need to be caught and Pig numbered error messages (which may just print 
the jodatime error message with a notification of which function it came from) 
will need to be added.

In ISOToX methods, the comments refer to rounding values.  But the code isn't 
rounding, it's truncating.


> ISO Date UDFs: Conversion, Rounding and Date Math
> -------------------------------------------------
>
>                 Key: PIG-1310
>                 URL: https://issues.apache.org/jira/browse/PIG-1310
>             Project: Pig
>          Issue Type: New Feature
>          Components: impl
>            Reporter: Russell Jurney
>             Fix For: 0.7.0
>
>         Attachments: datetime.patch, datetime2.patch
>
>   Original Estimate: 168h
>  Remaining Estimate: 168h
>
> I've written UDFs to handle loading unix times, datemonth values and ISO 8601 
> formatted date strings, and working with them as ISO datetimes using jodatime.
> The working code is here: 
> http://github.com/rjurney/oink/tree/master/src/java/oink/udf/isodate/
> It needs to be documented and tests added, and a couple UDFs are missing, but 
> these work if you REGISTER the jodatime jar in your script.  Hopefully I can 
> get this stuff in piggybank before someone else writes it this time :)  The 
> rounding also may not be performant, but the code works.
> Ultimately I'd also like to enable support for ISO 8601 durations.  Someone 
> slap me if this isn't done soon, it is not much work and this should help 
> everyone working with time series.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to