Skip to content
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

Handle DST changes in gapfill #6507

Merged
merged 1 commit into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .unreleased/fix_6507
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixes: #6507 time_bucket_gapfill with timezones doesn't handle daylight savings
Thanks: @JerkoNikolic Thanks for reporting the issue with gapfill and DST
30 changes: 26 additions & 4 deletions tsl/src/nodes/gapfill/gapfill_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -656,10 +656,32 @@ gapfill_advance_timestamp(GapFillState *state)
* To be consistent with time_bucket we do UTC bucketing unless
* a different timezone got explicity passed to the function.
*/
next = DirectFunctionCall2(state->have_timezone ? timestamptz_pl_interval :
timestamp_pl_interval,
TimestampTzGetDatum(state->gapfill_start),
IntervalPGetDatum(state->next_offset));
if (state->have_timezone)
{
bool isnull;
/* TODO: optimize by constifying and caching the datum if possible */
Datum tzname = gapfill_exec_expr(state, get_timezone_arg(state), &isnull);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opted for simple approach, we can optimize the performance if necessary later on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is needed for caching, something more than saving this datum in the GapFillState?

Assert(!isnull);

/* Convert to local timestamp */
next = DirectFunctionCall2(timestamptz_zone,
tzname,
TimestampTzGetDatum(state->gapfill_start));

/* Add interval */
next = DirectFunctionCall2(timestamp_pl_interval,
next,
IntervalPGetDatum(state->next_offset));

/* Convert back to specified timezone */
next = DirectFunctionCall2(timestamp_zone, tzname, next);
}
else
{
next = DirectFunctionCall2(timestamp_pl_interval,
TimestampTzGetDatum(state->gapfill_start),
IntervalPGetDatum(state->next_offset));
}
state->next_timestamp = DatumGetTimestampTz(next);
break;
default:
Expand Down
21 changes: 18 additions & 3 deletions tsl/test/shared/expected/gapfill-13.out
Original file line number Diff line number Diff line change
Expand Up @@ -3315,9 +3315,9 @@ SELECT time_bucket_gapfill('2 month'::interval, ts, 'UTC', '2000-01-01','2001-01
time_bucket_gapfill
Fri Dec 31 16:00:00 1999 PST
Tue Feb 29 16:00:00 2000 PST
Sun Apr 30 16:00:00 2000 PDT
Fri Jun 30 16:00:00 2000 PDT
Thu Aug 31 16:00:00 2000 PDT
Sun Apr 30 17:00:00 2000 PDT
Fri Jun 30 17:00:00 2000 PDT
Thu Aug 31 17:00:00 2000 PDT
Tue Oct 31 16:00:00 2000 PST
Sun Dec 31 16:00:00 2000 PST
(7 rows)
Expand Down Expand Up @@ -3391,6 +3391,21 @@ GROUP BY 1;
Fri Jun 30 15:00:00 2023 PDT |
(7 rows)

-- Test gapfill respects DST changes when generating timestamps (#6344)
SELECT time_bucket_gapfill('1 month', time, 'awst','2023-01-01', '2023-07-01' ) AS month, sum(value) AS sum
FROM month_timezone
GROUP BY 1;
month | sum
------------------------------+-------
Sat Dec 31 08:00:00 2022 PST |
Tue Jan 31 08:00:00 2023 PST |
Tue Feb 28 08:00:00 2023 PST | 3.123
Fri Mar 31 09:00:00 2023 PDT | 4.123
Sun Apr 30 09:00:00 2023 PDT | 5.123
Wed May 31 09:00:00 2023 PDT |
Fri Jun 30 09:00:00 2023 PDT |
(7 rows)

DROP TABLE month_timezone;
-- Test gapfill with additional group pathkeys added for optimization (#6396)
CREATE TABLE stocks_real_time (
Expand Down
21 changes: 18 additions & 3 deletions tsl/test/shared/expected/gapfill-14.out
Original file line number Diff line number Diff line change
Expand Up @@ -3315,9 +3315,9 @@ SELECT time_bucket_gapfill('2 month'::interval, ts, 'UTC', '2000-01-01','2001-01
time_bucket_gapfill
Fri Dec 31 16:00:00 1999 PST
Tue Feb 29 16:00:00 2000 PST
Sun Apr 30 16:00:00 2000 PDT
Fri Jun 30 16:00:00 2000 PDT
Thu Aug 31 16:00:00 2000 PDT
Sun Apr 30 17:00:00 2000 PDT
Fri Jun 30 17:00:00 2000 PDT
Thu Aug 31 17:00:00 2000 PDT
Tue Oct 31 16:00:00 2000 PST
Sun Dec 31 16:00:00 2000 PST
(7 rows)
Expand Down Expand Up @@ -3391,6 +3391,21 @@ GROUP BY 1;
Fri Jun 30 15:00:00 2023 PDT |
(7 rows)

-- Test gapfill respects DST changes when generating timestamps (#6344)
SELECT time_bucket_gapfill('1 month', time, 'awst','2023-01-01', '2023-07-01' ) AS month, sum(value) AS sum
FROM month_timezone
GROUP BY 1;
month | sum
------------------------------+-------
Sat Dec 31 08:00:00 2022 PST |
Tue Jan 31 08:00:00 2023 PST |
Tue Feb 28 08:00:00 2023 PST | 3.123
Fri Mar 31 09:00:00 2023 PDT | 4.123
Sun Apr 30 09:00:00 2023 PDT | 5.123
Wed May 31 09:00:00 2023 PDT |
Fri Jun 30 09:00:00 2023 PDT |
(7 rows)

DROP TABLE month_timezone;
-- Test gapfill with additional group pathkeys added for optimization (#6396)
CREATE TABLE stocks_real_time (
Expand Down
21 changes: 18 additions & 3 deletions tsl/test/shared/expected/gapfill-15.out
Original file line number Diff line number Diff line change
Expand Up @@ -3315,9 +3315,9 @@ SELECT time_bucket_gapfill('2 month'::interval, ts, 'UTC', '2000-01-01','2001-01
time_bucket_gapfill
Fri Dec 31 16:00:00 1999 PST
Tue Feb 29 16:00:00 2000 PST
Sun Apr 30 16:00:00 2000 PDT
Fri Jun 30 16:00:00 2000 PDT
Thu Aug 31 16:00:00 2000 PDT
Sun Apr 30 17:00:00 2000 PDT
Fri Jun 30 17:00:00 2000 PDT
Thu Aug 31 17:00:00 2000 PDT
Tue Oct 31 16:00:00 2000 PST
Sun Dec 31 16:00:00 2000 PST
(7 rows)
Expand Down Expand Up @@ -3391,6 +3391,21 @@ GROUP BY 1;
Fri Jun 30 15:00:00 2023 PDT |
(7 rows)

-- Test gapfill respects DST changes when generating timestamps (#6344)
SELECT time_bucket_gapfill('1 month', time, 'awst','2023-01-01', '2023-07-01' ) AS month, sum(value) AS sum
FROM month_timezone
GROUP BY 1;
month | sum
------------------------------+-------
Sat Dec 31 08:00:00 2022 PST |
Tue Jan 31 08:00:00 2023 PST |
Tue Feb 28 08:00:00 2023 PST | 3.123
Fri Mar 31 09:00:00 2023 PDT | 4.123
Sun Apr 30 09:00:00 2023 PDT | 5.123
Wed May 31 09:00:00 2023 PDT |
Fri Jun 30 09:00:00 2023 PDT |
(7 rows)

DROP TABLE month_timezone;
-- Test gapfill with additional group pathkeys added for optimization (#6396)
CREATE TABLE stocks_real_time (
Expand Down
21 changes: 18 additions & 3 deletions tsl/test/shared/expected/gapfill-16.out
Original file line number Diff line number Diff line change
Expand Up @@ -3317,9 +3317,9 @@ SELECT time_bucket_gapfill('2 month'::interval, ts, 'UTC', '2000-01-01','2001-01
time_bucket_gapfill
Fri Dec 31 16:00:00 1999 PST
Tue Feb 29 16:00:00 2000 PST
Sun Apr 30 16:00:00 2000 PDT
Fri Jun 30 16:00:00 2000 PDT
Thu Aug 31 16:00:00 2000 PDT
Sun Apr 30 17:00:00 2000 PDT
Fri Jun 30 17:00:00 2000 PDT
Thu Aug 31 17:00:00 2000 PDT
Tue Oct 31 16:00:00 2000 PST
Sun Dec 31 16:00:00 2000 PST
(7 rows)
Expand Down Expand Up @@ -3393,6 +3393,21 @@ GROUP BY 1;
Fri Jun 30 15:00:00 2023 PDT |
(7 rows)

-- Test gapfill respects DST changes when generating timestamps (#6344)
SELECT time_bucket_gapfill('1 month', time, 'awst','2023-01-01', '2023-07-01' ) AS month, sum(value) AS sum
FROM month_timezone
GROUP BY 1;
month | sum
------------------------------+-------
Sat Dec 31 08:00:00 2022 PST |
Tue Jan 31 08:00:00 2023 PST |
Tue Feb 28 08:00:00 2023 PST | 3.123
Fri Mar 31 09:00:00 2023 PDT | 4.123
Sun Apr 30 09:00:00 2023 PDT | 5.123
Wed May 31 09:00:00 2023 PDT |
Fri Jun 30 09:00:00 2023 PDT |
(7 rows)

DROP TABLE month_timezone;
-- Test gapfill with additional group pathkeys added for optimization (#6396)
CREATE TABLE stocks_real_time (
Expand Down
5 changes: 5 additions & 0 deletions tsl/test/shared/sql/gapfill.sql.in
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,11 @@ FROM
month_timezone
GROUP BY 1;

-- Test gapfill respects DST changes when generating timestamps (#6344)
SELECT time_bucket_gapfill('1 month', time, 'awst','2023-01-01', '2023-07-01' ) AS month, sum(value) AS sum
FROM month_timezone
GROUP BY 1;

DROP TABLE month_timezone;

-- Test gapfill with additional group pathkeys added for optimization (#6396)
Expand Down