-
Notifications
You must be signed in to change notification settings - Fork 14.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(presto/trino): Add TIME/TIMESTAMP WITH TIME ZONE #19263
fix(presto/trino): Add TIME/TIMESTAMP WITH TIME ZONE #19263
Conversation
e3fcaaf
to
54b5528
Compare
54b5528
to
49a1db9
Compare
Codecov Report
@@ Coverage Diff @@
## master #19263 +/- ##
=======================================
Coverage 66.53% 66.54%
=======================================
Files 1667 1667
Lines 64360 64362 +2
Branches 6493 6493
=======================================
+ Hits 42824 42827 +3
+ Misses 19854 19853 -1
Partials 1682 1682
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
49a1db9
to
a813bad
Compare
a813bad
to
299f6b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. BTW, Postgresql has timestamp with time zone
as well, we can add to pg specs in the future.
Co-authored-by: John Bodley <[email protected]>
Co-authored-by: John Bodley <[email protected]> (cherry picked from commit 82a6811)
SUMMARY
Presto (and thus Trino) have specific
TIME
andTIMESTAMP
types which support time zones. If the underlying column type isTIMESTAMP WITH TIME ZONE
then thePrestoEngineSpec.convert_dttm
method would incorrectly format the Pythondatatime
as a SQL string literal as opposed to casting it to aTIMESTAMP
.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Added unit tests and conformed in Presto:
from_iso8601_timestamp
UDF supported time zones, i.e.,from_iso8601_timestamp('2022-01-01T01:23:46.600+00:00')
TIMESTAMP
SELECT TIMESTAMP '2001-08-22 03:04:05.321 America/Los_Angeles' > TIMESTAMP '2001-08-22 03:04:05.321' returnedtrue
for a Presto server using a UTC-localized time zone.ADDITIONAL INFORMATION