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
Add compat setting for non-const timezones #50834
Merged
Merged
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
robot-ch-test-poll2
added
the
pr-bugfix
Pull request with bugfix, not backported by default
label
Jun 10, 2023
This is an automated comment for commit 6bdbcd3 with description of existing statuses. It's updated for the latest CI running
|
rschu1ze
force-pushed
the
non-const-tz
branch
3 times, most recently
from
June 10, 2023 16:42
ea35380
to
c0cfd88
Compare
rschu1ze
changed the title
(wip) Add compat setting for non-const timezones
Add compat setting for non-const timezones
Jun 10, 2023
SQL function toTimezone() converts a Date or DateTime into another timezone. The problem is that the timezone is part of the Date / DateTime type but not part of the internal representation (value). This led to the fact that toTimeZone() wqith non-const timezones produced wrong and misleading results until ClickHouse#48471 (shipped with v23.4) enforced a const timezone. Unfortunately, this PR also broke existing table definitions with non-const timezones, e.g. in ALIAS expressions. So while ClickHouse#48471 addressed the issue appropriately, it is really backwards-incompatible. This PR adds a setting to toggle the behavior and makes it also part of the compatibility profile.
Avogar
approved these changes
Jun 12, 2023
Should we backport it to 23.4 and 23.5? |
Yepp, that makes sense --> added the backport tag. |
rschu1ze
added
the
pr-must-backport
Pull request should be backported intentionally. Use this label with great care!
label
Jun 12, 2023
This was referenced Jun 12, 2023
robot-clickhouse
added a commit
that referenced
this pull request
Jun 12, 2023
rschu1ze
added a commit
to rschu1ze/ClickHouse
that referenced
this pull request
Jun 12, 2023
rschu1ze
added a commit
that referenced
this pull request
Jun 13, 2023
Backport #50834 to 23.5: Add compat setting for non-const timezones
robot-ch-test-poll3
added
the
pr-backports-created
Backport PRs are successfully created, it won't be processed by CI script anymore
label
Jun 13, 2023
rschu1ze
added a commit
to rschu1ze/ClickHouse
that referenced
this pull request
Jun 14, 2023
https://play.clickhouse.com/play?user=play#U0VMRUNUIGNoZWNrX25hbWUsIHRlc3RfbmFtZSwgcmVwb3J0X3VybApGUk9NIGNoZWNrcwpXSEVSRSAxCiAgICBBTkQgY2hlY2tfc3RhcnRfdGltZSA+PSBub3coKSAtIElOVEVSVkFMIDI0IEhPVVIKICAgIEFORCBwdWxsX3JlcXVlc3RfbnVtYmVyID0gMAogICAgQU5EIHRlc3Rfc3RhdHVzICE9ICdTS0lQUEVEJwogICAgQU5EIHRlc3Rfc3RhdHVzIExJS0UgJ0YlJwogICAgQU5EIGNoZWNrX3N0YXR1cyAhPSAnc3VjY2VzcycKT1JERVIgQlkgY2hlY2tfbmFtZSwgdGVzdF9uYW1lLCBjaGVja19zdGFydF90aW1l This is follow-up to ClickHouse#50834
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
pr-backports-created
Backport PRs are successfully created, it won't be processed by CI script anymore
pr-backports-created-cloud
pr-bugfix
Pull request with bugfix, not backported by default
pr-must-backport
Pull request should be backported intentionally. Use this label with great care!
pr-must-backport-cloud
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.
SQL function
toTimeZone()
converts a Date or DateTime into another timezone. The problem is that the timezone is part of theDate
/DateTime
type but not part of the value representation. This led to the fact thattoTimeZone()
with non-const timezones produced wrong and misleading results until #48471 (shipped with v23.4) enforced a const timezone.Unfortunately, #48471 also broke existing table definitions with non-const timezone expressions, e.g. in
ALIAS
. So while #48471 addressed the issue appropriately, it is backwards-incompatible.This PR adds setting
allow_nonconst_timezone_arguments
to achieve the old behavior fortoTimeZone()
and a few other functions + adds the setting to the compat profile.Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added setting allow_nonconst_timezone_arguments to toggle whether const time zone arguments in SQL functions are allowed or not