paul-rogers commented 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. 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-unsubscr...@drill.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
