PR #97 removed all datetime casts/type annotations from generated SQL for performance reasons. However, there was a miscommunication, and we discovered that SQL date/time functions require that their arguments be CAST'ed or Calcite throws an error.
This PR restores the cast to all datetime literals. The more sophisticated fix would be to only cast string literal function arguments. However, doing that would require we write a RegEx/parser to identify these arguments, then add a cast to them. This PR's implementation just restores the existing Date object formatter's former code (before PR #97) to prepend the cast to Date strings (and, therefore, we already know it works). Note that we retain PR #97's removal of CAST(... AS TIMESTAMP(3)) in SQL queries, which still gives us significant speed improvements over 4.8 (which make the slowdown from this PR insignificant).
Merge Checklist
:wrench: Issue(s) fixed:
[x] Author referenced issue(s) fixed by this PR:
[x] Fixes FE-9812
:smoking: Smoke Test
[x] Works in chrome
[ ] Works in firefox
[ ] Works in safari
[ ] Works in ie edge
[ ] Works in ie 11
:ship: Merge
[ ] author crafted PR's title into release-worthy commit message.
PR #97 removed all datetime casts/type annotations from generated SQL for performance reasons. However, there was a miscommunication, and we discovered that SQL date/time functions require that their arguments be
CAST'ed or Calcite throws an error.This PR restores the cast to all datetime literals. The more sophisticated fix would be to only cast string literal function arguments. However, doing that would require we write a RegEx/parser to identify these arguments, then add a cast to them. This PR's implementation just restores the existing
Dateobject formatter's former code (before PR #97) to prepend the cast to Date strings (and, therefore, we already know it works). Note that we retain PR #97's removal ofCAST(... AS TIMESTAMP(3))in SQL queries, which still gives us significant speed improvements over 4.8 (which make the slowdown from this PR insignificant).Merge Checklist
:wrench: Issue(s) fixed:
:smoking: Smoke Test
:ship: Merge