Add CAST TIMESTAMP WITH TIME ZONE to and from TIMESTAMP #6529
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
✅ Deploy Preview for meta-velox canceled.
|
facebook-github-bot
added
the
CLA Signed
zacw7
force-pushed
the
cast-ts-with-tz
branch
2 times, most recently
from
8e74401
to
f3928e6
Compare
zacw7
force-pushed
the
cast-ts-with-tz
branch
2 times, most recently
from
eeb2454
to
679a83c
Compare
mbasmanova
changed the title
mbasmanova
reviewed
Sep 12, 2023
There was a problem hiding this comment.
@zacw7 Zac, please, update the documentation to include the new CAST logic: https://facebookincubator.github.io/velox/functions/presto/conversion.html
mbasmanova
reviewed
Sep 12, 2023
mbasmanova
reviewed
Sep 12, 2023
mbasmanova
changed the title
zacw7
force-pushed
the
cast-ts-with-tz
branch
2 times, most recently
from
f7582a4
to
cf6dced
Compare
|
@zacw7 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
reviewed
Sep 12, 2023
zacw7
force-pushed
the
cast-ts-with-tz
branch
3 times, most recently
from
9d3eadf
to
07d05fb
Compare
|
@zacw7 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
zacw7
force-pushed
the
cast-ts-with-tz
branch
from
07d05fb
to
18b789f
Compare
|
@zacw7 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
reviewed
Sep 13, 2023
There was a problem hiding this comment.
@zacw7 Thank you for adding detailed documentation. This is very helpful. Appreciate your effort.
Overall looks good % a few minor comments.
velox/functions/prestosql/tests/TimestampWithTimeZoneCastTest.cpp
Outdated
Show resolved
Hide resolved
velox/functions/prestosql/tests/TimestampWithTimeZoneCastTest.cpp
Outdated
Show resolved
Hide resolved
mbasmanova
changed the title
zacw7
force-pushed
the
cast-ts-with-tz
branch
3 times, most recently
from
0aca69d
to
3b3f0e8
Compare
|
@zacw7 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
mbasmanova
approved these changes
Sep 13, 2023
There was a problem hiding this comment.
@zacw7 Thank you.
@laithsakka @kagamiori Laith, Wei, please, take a look as well.
| SELECT cast(from_unixtime(0, '+06:00') as timestamp); -- 1970-01-01 06:00:00.000 | ||
| SELECT cast(from_unixtime(0, '-02:00') as timestamp); -- 1969-12-31 22:00:00.000 | ||
|
|
||
| Cast to Timestamp with Time Zone |
There was a problem hiding this comment.
TIMESTAMP WITH TIME ZONE
|
|
||
| auto timestampVector = rowResult.childAt(0)->asFlatVector<int64_t>(); | ||
| auto timezoneVector = rowResult.childAt(1)->asFlatVector<int16_t>(); | ||
| auto rawTsValues = timestampVector->values()->asMutable<int64_t>(); |
There was a problem hiding this comment.
@zacw7 Please, double check.
zacw7
force-pushed
the
cast-ts-with-tz
branch
from
3b3f0e8
to
997441a
Compare
This change adds casting support for timestamp with time zone. Currently, it only supports cast timestamp with time zone from and to timestamp. Presto also allows casting from/to Date and Varchar, which are not included in the current change.
zacw7
force-pushed
the
cast-ts-with-tz
branch
from
997441a
to
0af11d9
Compare
|
@zacw7 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
codyschierbeck
pushed a commit
to codyschierbeck/velox
that referenced
this pull request
Sep 27, 2023
…bator#6529) Summary: Presto also allows CAST TIMESTAMP WITH TIME ZONE to and from DATE and VARCHAR. This functionality is not available yet. Pull Request resolved: facebookincubator#6529 Reviewed By: mbasmanova Differential Revision: D49196796 Pulled By: zacw7 fbshipit-source-id: 57dff733fda2320c1f7ee2a09a35d8ea7823fa8f
codyschierbeck
pushed a commit
to codyschierbeck/velox
that referenced
this pull request
Sep 27, 2023
…bator#6529) Summary: Presto also allows CAST TIMESTAMP WITH TIME ZONE to and from DATE and VARCHAR. This functionality is not available yet. Pull Request resolved: facebookincubator#6529 Reviewed By: mbasmanova Differential Revision: D49196796 Pulled By: zacw7 fbshipit-source-id: 57dff733fda2320c1f7ee2a09a35d8ea7823fa8f
codyschierbeck
pushed a commit
to codyschierbeck/velox
that referenced
this pull request
Sep 27, 2023
…bator#6529) Summary: Presto also allows CAST TIMESTAMP WITH TIME ZONE to and from DATE and VARCHAR. This functionality is not available yet. Pull Request resolved: facebookincubator#6529 Reviewed By: mbasmanova Differential Revision: D49196796 Pulled By: zacw7 fbshipit-source-id: 57dff733fda2320c1f7ee2a09a35d8ea7823fa8f
ericyuliu
pushed a commit
to ericyuliu/velox
that referenced
this pull request
Oct 12, 2023
…bator#6529) Summary: Presto also allows CAST TIMESTAMP WITH TIME ZONE to and from DATE and VARCHAR. This functionality is not available yet. Pull Request resolved: facebookincubator#6529 Reviewed By: mbasmanova Differential Revision: D49196796 Pulled By: zacw7 fbshipit-source-id: 57dff733fda2320c1f7ee2a09a35d8ea7823fa8f
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.
Presto also allows CAST TIMESTAMP WITH TIME ZONE to and from DATE and VARCHAR.
This functionality is not available yet.