Implement the timezoneOffset function #19962
Conversation
robot-clickhouse
added
doc-alert
pr-feature
| @@ -274,6 +274,23 @@ class DateLUTImpl | |||
| return res / 3600; | |||
| } | |||
|
|
|||
|
|
|||
| inline time_t timezoneOffset(time_t t) const | |||
There was a problem hiding this comment.
Comment to function is needed.
|
|
||
| /// Calculate daylight saving offset first, ignore the leap seconds | ||
| time_t res = (lut[index].date - lut[0].date) % 86400; | ||
| res = res > 43200 ? (86400 - res) : (0 - res); |
There was a problem hiding this comment.
The calculation seem to be correct, but it is better to add comment about why so.
There was a problem hiding this comment.
yes, I've appended comments.
src/Functions/DateTimeTransforms.h
Outdated
| return dateIsNotSupported(name); | ||
| } | ||
| /////need to do | ||
| using FactorTransform = ToDateImpl; |
There was a problem hiding this comment.
Unfortunately, we even can't use ToDateImpl because this function is not positively monotonic (as assumed for other transforms). We should update getMonotonicityForRange
or just use
ToTimeImpl for now.
There was a problem hiding this comment.
I use ToTimeImpl now.
src/Functions/DateTimeTransforms.h
Outdated
| @@ -407,6 +407,23 @@ struct ToHourImpl | |||
| using FactorTransform = ToDateImpl; | |||
| }; | |||
|
|
|||
| struct timezoneOffsetImpl | |||
| /* timestamp == (Europe/Moscow) */ | ||
|
|
||
|
|
||
| SELECT number,(toDateTime('1981-04-01 00:00:00', 'Europe/Moscow') + INTERVAL number * 600 SECOND) AS k, timezoneOffset(k) AS t, toUnixTimestamp(k) as s FROM numbers(200); |
There was a problem hiding this comment.
It would be good if test result is readable. Now, when it has > 1k rows it is not. Can we just use, e.g., 2 rows instead of 200?
There was a problem hiding this comment.
The most interesting cases are the cases when daylight saving time have changed. With time just before and just after change. Also compare result with year with permanent DST (e.g. from 2011 to 2014 in Russia). Please, add comment if needed.
There was a problem hiding this comment.
have made modifications based on your opinion
thanx~
|
by the way, during the test I noticed some unexpected results shown as below. I think the problem may come from the toDateTime function when processing the special timezone of 'Australia/Lord_Howe', there're many strange time skips near the DST change. need to be verified. |
|
Looks like a bug indeed, but if it is, we'd better fix it in another pr. |
that's ok |
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added timezoneOffset(datetime) function which will give the offset from UTC in seconds. This close #issue:19850.
Detailed description / Documentation draft:
Implementing this by adding a method in DateLUTImpl, we calculate the offset by consider timezone epoch also with the daylight saving change, the leap seconds is not involved now. This function may get the same result as dateDiff('second', toDateTime('somedatetime', 'time_zone_name'), toDateTime('somedatetime','UTC')). And the times that fall in the exactly changing belt which can’t be expressed by the formatted date string is also under consideration.