fix: Casting Varchar to Timestamp should handle offsets that are not recognized time zones #11849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
|
This pull request was exported from Phabricator. Differential Revision: D67182709 |
kevinwilfong
pushed a commit
to kevinwilfong/velox
that referenced
this pull request
Dec 13, 2024
…recognized time zones (facebookincubator#11849) Summary: When casting a Varchar to Timestamp, Presto Java allows an offset timestamp in place of a time zone. This string is of the form +/-HH:MM:SS.mmm where all units except the hour are optional, the colons are optional, the . is optional and may also be a , and this string must be separated by the date/time by a single space. This is not interpreted as a time zone but rather simply a milliseconds from UTC and is applied in addition to the time zone conversion if a session time zone is present. Note that strings that fit this pattern but are valid time zones e.g. +02:00 are still treated as time zones. Since this is not a true time zone it is not allowed when casting from Varchar to TimestampWithTimeZone or any other string to time conversions (at least AFAICT). This change updates fromTimestampWithTimezoneString to handle this case and return an offsetMillis in place of a time zone if one is present. Casting from Varchar to Timestamp has been updated to apply this offset to the result, while all other locations where this function is called have been updated to throw if offsetMillis is present. Differential Revision: D67182709
kevinwilfong
force-pushed
the
export-D67182709
branch
from
c0ea819 to
90954b4
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67182709 |
…recognized time zones (facebookincubator#11849) Summary: When casting a Varchar to Timestamp, Presto Java allows an offset timestamp in place of a time zone. This string is of the form +/-HH:MM:SS.mmm where all units except the hour are optional, the colons are optional, the . is optional and may also be a , and this string must be separated by the date/time by a single space. This is not interpreted as a time zone but rather simply a milliseconds from UTC and is applied in addition to the time zone conversion if a session time zone is present. Note that strings that fit this pattern but are valid time zones e.g. +02:00 are still treated as time zones. Since this is not a true time zone it is not allowed when casting from Varchar to TimestampWithTimeZone or any other string to time conversions (at least AFAICT). This change updates fromTimestampWithTimezoneString to handle this case and return an offsetMillis in place of a time zone if one is present. Casting from Varchar to Timestamp has been updated to apply this offset to the result, while all other locations where this function is called have been updated to throw if offsetMillis is present. Differential Revision: D67182709
kevinwilfong
force-pushed
the
export-D67182709
branch
from
90954b4 to
671ea44
Compare
|
This pull request was exported from Phabricator. Differential Revision: D67182709 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
When casting a Varchar to Timestamp, Presto Java allows an offset timestamp in place of a time zone. This
string is of the form +/-HH:MM:SS.mmm where all units except the hour are optional, the colons are optional,
the . is optional and may also be a , and this string must be separated by the date/time by a single space.
This is not interpreted as a time zone but rather simply a milliseconds from UTC and is applied in addition to the
time zone conversion if a session time zone is present. Note that strings that fit this pattern but are valid time
zones e.g. +02:00 are still treated as time zones.
Since this is not a true time zone it is not allowed when casting from Varchar to TimestampWithTimeZone or any
other string to time conversions (at least AFAICT).
This change updates fromTimestampWithTimezoneString to handle this case and return an offsetMillis in place
of a time zone if one is present. Casting from Varchar to Timestamp has been updated to apply this offset to
the result, while all other locations where this function is called have been updated to throw if offsetMillis is
present.
Differential Revision: D67182709