Skip to content

01099_operators_date_and_timestamp: Use dates that work with all available timezones#27275

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
Algunenano:fix_01099
Aug 6, 2021
Merged

01099_operators_date_and_timestamp: Use dates that work with all available timezones#27275
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
Algunenano:fix_01099

Conversation

@Algunenano
Copy link
Copy Markdown
Member

@Algunenano Algunenano commented Aug 6, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
...

Detailed description / Documentation draft:

It appeared in #27235 CI

This test would fail when running under the Asia/Amman timezone and Africa/Cairo / Egypt. Change the day so it works with all 594 currently available timezones.

I was using something like this one to test the values:

Select tz
FROM
(
Select 'Africa/Abidjan' AS tz, toDateTime('2001-09-29', 'Africa/Abidjan') - Interval 1 hour as t, toDateTime('2001-09-28 23:00:00', 'Africa/Abidjan') as t2 UNION ALL
Select 'Africa/Accra' AS tz, toDateTime('2001-09-29', 'Africa/Accra') - Interval 1 hour as t, toDateTime('2001-09-28 23:00:00', 'Africa/Accra') as t2 UNION ALL
...
Select 'Zulu' AS tz, toDateTime('2001-09-29', 'Zulu') - Interval 1 hour as t, toDateTime('2001-09-28 23:00:00', 'Zulu') as t2
)
where t != t2

So it isn't automatable

@robot-clickhouse robot-clickhouse added the pr-not-for-changelog This PR should not be mentioned in the changelog label Aug 6, 2021
@azat
Copy link
Copy Markdown
Member

azat commented Aug 6, 2021

I saw similar failure in one of my patches.
But is this correct to use static timezones here? I.e. why + 1 hour does not work for Asia/Amman?

SELECT toDateTime('2001-09-28 00:00:00', 'Asia/Amman') + toIntervalHour(1)

Query id: 76618a2f-bc0a-4389-9365-7d7005e583ea

┌─plus(toDateTime('2001-09-28 00:00:00', 'Asia/Amman'), toIntervalHour(1))─┐
│                                                      2001-09-28 00:00:00 │
└──────────────────────────────────────────────────────────────────────────┘

@Algunenano
Copy link
Copy Markdown
Member Author

Algunenano commented Aug 6, 2021

But is this correct to use static timezones here? I.e. why + 1 hour does not work for Asia/Amman?

Timezone change (from +3 to +2):

SELECT timezoneOffset(toDateTime('2001-09-28', 'Asia/Amman') + toIntervalHour(number))
FROM system.numbers
LIMIT 10

Query id: 4f927d69-8c53-42d7-87d5-7c2d4036a9e5

┌─timezoneOffset(plus(toDateTime('2001-09-28', 'Asia/Amman'), toIntervalHour(number)))─┐
│                                                                                10800 │
│                                                                                 7200 │
│                                                                                 7200 │
│                                                                                 7200 │
│                                                                                 7200 │
│                                                                                 7200 │
│                                                                                 7200 │
│                                                                                 7200 │
│                                                                                 7200 │
│                                                                                 7200 │
└──────────────────────────────────────────────────────────────────────────────────────┘

That is, it worked, but at 01:00 they moved back to 00:00. See https://www.timeanddate.com/time/change/jordan/amman?year=2001

@azat azat mentioned this pull request Aug 6, 2021
Merged
@alexey-milovidov alexey-milovidov self-assigned this Aug 6, 2021
@alexey-milovidov alexey-milovidov merged commit ef7b695 into ClickHouse:master Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

Assignees

@alexey-milovidov alexey-milovidov

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Algunenano @azat @alexey-milovidov @robot-clickhouse