-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix time_bucket DST handling with timezone and offset #9129
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9129 +/- ##
==========================================
- Coverage 82.46% 82.30% -0.17%
==========================================
Files 243 244 +1
Lines 47938 46866 -1072
Branches 12234 12230 -4
==========================================
- Hits 39534 38574 -960
+ Misses 3536 3481 -55
+ Partials 4868 4811 -57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fbd83be to
a6be389
Compare
8da65e1 to
fffd7b8
Compare
When both timezone and offset parameters were specified, the offset was incorrectly applied to local time after timezone conversion. This could create non-existent times during DST transitions. For example, in Asia/Amman on 2021-03-26, clocks jumped from 00:00 to 01:00, skipping the entire 00:00-00:59 hour. Subtracting 15 minutes from 01:00 local time would produce 00:45, which doesn't exist. The fix applies the offset in UTC space before timezone conversion, ensuring DST transitions are handled correctly. Fixes #7059
fffd7b8 to
e463b70
Compare
Update expected output files to match actual time_bucket function results when using timezone and offset parameters across DST transitions.
|
Looks that it also fixes this one #9136 ? |
Yeah, looks related. We should add the test in that issue to this PR. @tjgreen42 |
Slightly different case actually -- this PR fixes the situation for |
During DST fall-back, PostgreSQL's timestamp_zone picks the standard time interpretation when converting an ambiguous local time back to timestamptz. If the original timestamp was in daylight time, the bucket could incorrectly start AFTER the timestamp. Fix by subtracting periods until the bucket is at or before the timestamp. Also add test case for issue #9136. Fixes #9136
| @@ -0,0 +1 @@ | |||
| Fixes: #7059 #8851 #9136 Fix time_bucket with timezone during DST | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is supposed to be the PR number since we compile changelog from this file, the actual bugs should go into commit message/linked to PR. Any external contributors reporting any of the bugs should get a thank you line
| @@ -0,0 +1 @@ | |||
| Fixes: #7059 #8851 #9136 Fix time_bucket with timezone during DST | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Fixes: #7059 #8851 #9136 Fix time_bucket with timezone during DST | |
| Fixes: #9129 Fix time_bucket with timezone during DST | |
| Thanks: @cracksalad and @eyadmba for reporting a bug with timezone handling in time_bucket |
Summary
Fix
time_bucket()with timezone parameter during DST transitions:Problems Fixed
#7059: Offset + timezone during DST spring-forward
The offset was applied to local time after timezone conversion, which could create non-existent times.
Example (Asia/Amman): On 2021-03-26, clocks jumped from 00:00 to 01:00. Subtracting 15 minutes from 01:00 local produced 00:45, which doesn't exist.
#8851: Offset + timezone during DST fall-back
Same root cause as #7059, causing incorrect timestamps around fall-back transitions.
#9136: Origin + timezone during DST fall-back
When using
originparameter during DST fall-back, the bucket could start AFTER the input timestamp. Postgres picks the standard time interpretation for ambiguous local times, but if the input was in daylight time, the bucket ended up in the future.Solution
Testing
Added regression tests for DST boundary cases from all three issues.
Fixes #7059
Fixes #8851
Fixes #9136