[GitHub] [drill] paul-rogers edited a comment on pull request #2268: DRILL-7926: Age function fix

[GitBox <gi...@apache.org>]

paul-rogers edited a comment on pull request #2268:
URL: https://github.com/apache/drill/pull/2268#issuecomment-885869395


   @dzamo, thanks much for inspecting the code and for the tests. @oleg-zinovev, the reason you are dealing with this mess is that Drill clearly did not have adequate tests in this area. Please, as part of your work, go ahead and add sufficient unit tests to check for the cases which @dzamo showed and for the other cases in my comments.
   
   You may find that things don't work because of bugs in other areas. You seemed to find places where Drill deals with TIMESTAMP in UTC. As explained above, those are bugs: the bulk of Drill is written (alas) to assume TIMESTAMP is local time.
   
   `AGE()` is particularly nasty since it relies on the query start time, which will differ for each test. By factoring the logic into a function, where "now" is a parameter, you can write tests independent of the current time by passing in a known, fixed value for "now." Otherwise, the results of the tests will reflect the actual query start time, and perhaps even time zone, and the test driver will have to repeat the math to check the answer. Doable, but complex.
   
   If it were me, I would implement the age function as a normal Java function, test the heck out of it, then wrap it in a UDF. However, if you prefer, you can implement the logic in the UDF, then test the cases by running queries, which requires that you have a working reader and working conversion from Drill's output to some date/time class. Since I suspect these may also be buggy, doing "full up" testing may be a bit frustrating. Still, your choice.


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