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 12, 2023
1 parent 4c3d64a commit 1bd5273
Show file tree
Hide file tree
Showing 16 changed files with 3,893 additions and 341 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
17 changes: 10 additions & 7 deletions tsl/src/continuous_aggs/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -582,21 +582,24 @@ 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 = *bucket_info.interval;

/*
* 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

0 comments on commit 1bd5273

Please sign in to comment.