dzamo commented on pull request #2284: URL: https://github.com/apache/drill/pull/2284#issuecomment-892550419
Okay... 1. I converted the `AGE` function to a Freemarker template, removing duplicated code. 2. I got rid of some unnecessary time zone wrangling. I was trying to do everything using ZonedDateTimes in the configured time zone, because of worries about edge cases like daylight savings transitions and what effect people might expect those to have on `AGE`, but I then reviewed the `TIMESTAMPDIFF` implementation, a function that tackles a similar problem, and ended up calculating only with LocalDateTimes, consistent with `TIMESTAMPDIFF`. 3. The basic solution is Oleg's final Java Time API one, which is I think is very clean, and it works. The issue I thought it had over midnights was a red herring due to the string literals I was testing with get truncated to dates for lacking a `timestamp` prefix. So really the only thing wrong with where Oleg got to was the time zone handling of the query start time used in the unary form of `AGE`. 4. I've added tests, mainly for the unary form. 5. The Java Time techniques used here present an opportunity to considerably simplify the Java Time part of `TIMESTAMPDIFF`, so I did that. It's in a separate commit and doesn't have to be part of this PR if that would be cheating on paperwork. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
