Skip to content

Commit

Permalink
Rectify interval calculation
Browse files Browse the repository at this point in the history
For continuous aggregates with variable bucket size, the interval
was wrongly manipulated in the process. Now it is corrected by
creating a copy of interval structure for validation purposes
and keeping the original structure untouched.

Fixes #5734
  • Loading branch information
RafiaSabih committed Jul 11, 2023
1 parent eaa1206 commit 426650b
Show file tree
Hide file tree
Showing 16 changed files with 3,299 additions and 1,319 deletions.
5 changes: 5 additions & 0 deletions .unreleased/bugfix_5734
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Implements: #5860 Rectifies interval calculation for Heirarchical CAggs

Fixes: #5734

Thanks: @lukaskirner
6 changes: 6 additions & 0 deletions test/sql/updates/post.repair.hierarchical_cagg.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- This file and its contents are licensed under the Apache License 2.0.
-- Please see the included NOTICE for copyright information and
-- LICENSE-APACHE for a copy of the license.

SELECT count(*) FROM agg_test_monthly;

3 changes: 2 additions & 1 deletion test/sql/updates/post.repair.sql
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ WHERE extname = 'timescaledb' \gset

\if :test_repair_cagg_joins
--Check if the repaired cagg with joins work alright now
\ir post.repair.cagg_joins.sql
\ir post.repair.cagg_joins.sql
\ir post.repair.hierarchical_cagg.sql
\endif

46 changes: 46 additions & 0 deletions test/sql/updates/setup.repair.hierarchical_cagg.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
-- This file and its contents are licensed under the Apache License 2.0.
-- Please see the included NOTICE for copyright information and
-- LICENSE-APACHE for a copy of the license.

CREATE TABLE test (
timestamp TIMESTAMPTZ NOT NULL,
device_id TEXT NOT NULL,
value INT NOT NULL,

CONSTRAINT uk_test_timestamp_device_id UNIQUE (timestamp, device_id)
);

SELECT create_hypertable('test', 'timestamp');

INSERT INTO test (timestamp, device_id, value) VALUES
('2023-05-01 00:00:00+00', 'sensor0', 1),
('2023-05-15 00:00:00+00', 'sensor0', 2),
('2023-05-31 00:00:00+00', 'sensor0', 10);


CREATE MATERIALIZED VIEW agg_test_hourly WITH (timescaledb.continuous) AS
SELECT
time_bucket('1 hour'::interval, timestamp) AS hour_timestamp,
device_id,
SUM(value)
FROM test
GROUP BY hour_timestamp, device_id
WITH DATA;

CREATE MATERIALIZED VIEW agg_test_daily WITH (timescaledb.continuous) AS
SELECT
time_bucket('1 day'::interval, hour_timestamp) AS day_timestamp,
device_id,
SUM(sum)
FROM agg_test_hourly
GROUP BY day_timestamp, device_id
WITH DATA;

CREATE MATERIALIZED VIEW agg_test_monthly WITH (timescaledb.continuous) AS
SELECT
time_bucket('1 month'::interval, day_timestamp) AS month_timestamp,
device_id,
SUM(sum)
FROM agg_test_daily
GROUP BY month_timestamp, device_id
WITH DATA;
2 changes: 1 addition & 1 deletion test/sql/updates/setup.repair.sql
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ DROP VIEW slices;
\endif

\ir setup.repair.cagg.sql

\if :has_cagg_joins
\ir setup.repair.hierarchical_cagg.sql
\ir setup.repair.cagg_joins.sql
\endif
19 changes: 12 additions & 7 deletions tsl/src/continuous_aggs/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -582,21 +582,26 @@ get_bucket_width(CAggTimebucketInfo bucket_info)
break;
case INTERVALOID:
{
/*
* Original interval should not be changed, hence create a local copy
* for this check.
*/
Interval interval = { .time = bucket_info.interval->time,
.day = bucket_info.interval->day,
.month = bucket_info.interval->month };

/*
* epoch will treat year as 365.25 days. This leads to the unexpected
* result that year is not multiple of day or month, which is perceived
* as a bug. For that reason, we treat all months as 30 days regardless of year
*/
if (bucket_info.interval->month && !bucket_info.interval->day &&
!bucket_info.interval->time)
if (interval.month && !interval.day && !interval.time)
{
bucket_info.interval->day = bucket_info.interval->month * DAYS_PER_MONTH;
bucket_info.interval->month = 0;
interval.day = interval.month * DAYS_PER_MONTH;
interval.month = 0;
}

/* Convert Interval to int64 */
width =
ts_interval_value_to_internal(IntervalPGetDatum(bucket_info.interval), INTERVALOID);
width = ts_interval_value_to_internal(IntervalPGetDatum(&interval), INTERVALOID);
break;
}
default:
Expand Down
Loading

0 comments on commit 426650b

Please sign in to comment.