From d8057c432625981ecc44db960b72f394cabd3b35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADzio=20de=20Royes=20Mello?= Date: Fri, 3 May 2024 16:33:47 -0300 Subject: [PATCH] Fix policy refresh window too small When adding a CAgg refresh policy using `start_offset => '2 months'` and `end_offset => '0 months'` was failing because the policy refresh window is too small and it should cover at least two buckets in the valid time range of timestamptz. The problem was when calculating the bucket width for the variable bucket size (like months) we're assuming 31 days for each month but when converting the end offset to integer using `interval_to_int64` we use 30 days per month. Fixed it by aligning the variable bucket size calculation to also use 30 days. --- tsl/src/bgw_policy/continuous_aggregate_api.c | 4 +-- tsl/test/expected/exp_cagg_monthly.out | 29 ++++++++++++++++++- tsl/test/sql/exp_cagg_monthly.sql | 21 ++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/tsl/src/bgw_policy/continuous_aggregate_api.c b/tsl/src/bgw_policy/continuous_aggregate_api.c index f967a5e2655..ce4af4bb8af 100644 --- a/tsl/src/bgw_policy/continuous_aggregate_api.c +++ b/tsl/src/bgw_policy/continuous_aggregate_api.c @@ -443,7 +443,7 @@ validate_window_size(const ContinuousAgg *cagg, const CaggPolicyConfig *config) * 2. Buckets with timezones * 3. Cases 1 and 2 at the same time * - * For months we simply take 31 days as the worst case scenario and + * For months we simply take 30 days like on interval_to_int64 and * multiply this number by the number of months in the bucket. This * reduces the task to days/hours/minutes scenario. * @@ -459,7 +459,7 @@ validate_window_size(const ContinuousAgg *cagg, const CaggPolicyConfig *config) /* Make a temporary copy of bucket_width */ Interval interval = *cagg->bucket_function->bucket_time_width; - interval.day += 31 * interval.month; + interval.day += 30 * interval.month; interval.month = 0; bucket_width = ts_interval_value_to_internal(IntervalPGetDatum(&interval), INTERVALOID); } diff --git a/tsl/test/expected/exp_cagg_monthly.out b/tsl/test/expected/exp_cagg_monthly.out index 03cbe2278ac..d2d85aac584 100644 --- a/tsl/test/expected/exp_cagg_monthly.out +++ b/tsl/test/expected/exp_cagg_monthly.out @@ -1166,9 +1166,36 @@ ERROR: policy refresh window too small SELECT add_continuous_aggregate_policy('conditions_summary_policy', start_offset => INTERVAL '65 days', end_offset => INTERVAL '1 day', + schedule_interval => INTERVAL '1 hour') AS job_id \gset +SELECT delete_job(:job_id); + delete_job +------------ + +(1 row) + +SELECT add_continuous_aggregate_policy('conditions_summary_policy', + start_offset => INTERVAL '2 months', + end_offset => INTERVAL '0 months', + schedule_interval => INTERVAL '1 hour') AS job_id \gset +SELECT delete_job(:job_id); + delete_job +------------ + +(1 row) + +\set ON_ERROR_STOP 0 +SELECT add_continuous_aggregate_policy('conditions_summary_policy', + start_offset => INTERVAL '2 months', + end_offset => INTERVAL '1 months', + schedule_interval => INTERVAL '1 hour'); +ERROR: policy refresh window too small +\set ON_ERROR_STOP 1 +SELECT add_continuous_aggregate_policy('conditions_summary_policy', + start_offset => INTERVAL '3 months', + end_offset => INTERVAL '1 months', schedule_interval => INTERVAL '1 hour'); add_continuous_aggregate_policy --------------------------------- - 1000 + 1002 (1 row) diff --git a/tsl/test/sql/exp_cagg_monthly.sql b/tsl/test/sql/exp_cagg_monthly.sql index dd792ce1fd6..ddfcb25d325 100644 --- a/tsl/test/sql/exp_cagg_monthly.sql +++ b/tsl/test/sql/exp_cagg_monthly.sql @@ -544,4 +544,25 @@ SELECT add_continuous_aggregate_policy('conditions_summary_policy', SELECT add_continuous_aggregate_policy('conditions_summary_policy', start_offset => INTERVAL '65 days', end_offset => INTERVAL '1 day', + schedule_interval => INTERVAL '1 hour') AS job_id \gset + +SELECT delete_job(:job_id); + +SELECT add_continuous_aggregate_policy('conditions_summary_policy', + start_offset => INTERVAL '2 months', + end_offset => INTERVAL '0 months', + schedule_interval => INTERVAL '1 hour') AS job_id \gset + +SELECT delete_job(:job_id); + +\set ON_ERROR_STOP 0 +SELECT add_continuous_aggregate_policy('conditions_summary_policy', + start_offset => INTERVAL '2 months', + end_offset => INTERVAL '1 months', + schedule_interval => INTERVAL '1 hour'); +\set ON_ERROR_STOP 1 + +SELECT add_continuous_aggregate_policy('conditions_summary_policy', + start_offset => INTERVAL '3 months', + end_offset => INTERVAL '1 months', schedule_interval => INTERVAL '1 hour');