drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [drill] dzamo commented on pull request #2284: DRILL-7926: Age Function Fix
Date Thu, 05 Aug 2021 11:58:31 GMT

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: dev-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message