Skip to content

Saturate date/datetime to zero (part 2)#36656

Merged
vdimir merged 1 commit intoClickHouse:masterfrom
amosbird:timefunctionunderflow
May 2, 2022
Merged

Saturate date/datetime to zero (part 2)#36656
vdimir merged 1 commit intoClickHouse:masterfrom
amosbird:timefunctionunderflow

Conversation

@amosbird
Copy link
Copy Markdown
Collaborator

For partial hours/minutes timezones.

Changelog category (leave one):

  • Improvement

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

Now date time conversion functions that generates time before 1970-01-01 00:00:00 with partial hours/minutes timezones will be saturated to zero instead of overflow. This is the continuation of #29953 which addresses #29953 (comment) . Mark as improvement because it's implementation defined behavior (and very rare case) and we are allowed to break it.

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

@vdimir vdimir self-assigned this Apr 26, 2022
@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Apr 26, 2022
@vdimir
Copy link
Copy Markdown
Member

vdimir commented Apr 28, 2022

@amosbird test 01262_fractional_timezone_near_start_of_epoch failed with unexpected change:

SELECT toDateTime(10000, 'Asia/Calcutta');
SELECT toMinute(toDateTime(10000, 'Asia/Calcutta'));
SELECT toStartOfHour(toDateTime(10000, 'Asia/Calcutta'));
1970-01-01 08:16:40
16
-1970-01-01 08:00:00
+1970-01-02 00:00:00

@amosbird amosbird force-pushed the timefunctionunderflow branch 2 times, most recently from f091f46 to 4d09021 Compare April 29, 2022 02:21
@amosbird
Copy link
Copy Markdown
Collaborator Author

@amosbird test 01262_fractional_timezone_near_start_of_epoch failed with unexpected change:

SELECT toDateTime(10000, 'Asia/Calcutta');
SELECT toMinute(toDateTime(10000, 'Asia/Calcutta'));
SELECT toStartOfHour(toDateTime(10000, 'Asia/Calcutta'));
1970-01-01 08:16:40
16
-1970-01-01 08:00:00
+1970-01-02 00:00:00

I've switched to a different implementation which saturates those date time values more naturally.

For partial hours/minutes timezones.
@amosbird amosbird force-pushed the timefunctionunderflow branch from 4d09021 to e81929a Compare April 29, 2022 02:24
@vdimir vdimir merged commit 7293a69 into ClickHouse:master May 2, 2022
azat added a commit to azat/ClickHouse that referenced this pull request Aug 10, 2022
… flakiness

It is possible for toStartOfMinute() to give different result for 0 and
59, for partial timezones (timezone that does not starts from 00:00,
like Africa/Monrovia).

Before ClickHouse#36656 it fails for another reason, because of overflows [1], but
now it fails because it simply return different minutes.

  [1]: ClickHouse#29953 (comment)

Simply pin the UTC there.

Fixes: ClickHouse#37786
Signed-off-by: Azat Khuzhin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants