From 676b5a70e5fdb1ec161228e285d2e9124eb042d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADzio=20de=20Royes=20Mello?= Date: Wed, 26 Apr 2023 09:02:57 -0300 Subject: [PATCH] Fix cagg_repair for the old CAgg format In commit 4a6650d1 we fixed the cagg_repair running for broken Continuous Aggregates with JOINs but we accidentally removed the code path for running against the old format (finalized=false) leading us to a dead code pointed out by CoverityScan: https://scan4.scan.coverity.com/reports.htm#v54116/p12995/fileInstanceId=131706317&defectInstanceId=14569420&mergedDefectId=384044 Fixed it by restoring the old code path for running the cagg_repair for Continuous Aggregates in the old format (finalized=false). (cherry picked from commit 3c8d7cef77d34d88ace3cd0a58fbb7640965d5c0) --- tsl/src/continuous_aggs/create.c | 4 +-- tsl/test/expected/cagg_repair.out | 45 +++++++++++++++++++++++++++++-- tsl/test/sql/cagg_repair.sql | 18 +++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c index 365e6784dd1..326e33053a0 100644 --- a/tsl/src/continuous_aggs/create.c +++ b/tsl/src/continuous_aggs/create.c @@ -3045,7 +3045,7 @@ cagg_rebuild_view_definition(ContinuousAgg *agg, Hypertable *mat_ht, bool force_ } } - if (!rebuild_cagg_with_joins) + if (!rebuild_cagg_with_joins && finalized) { /* There's nothing to fix, so no need to rebuild */ elog(DEBUG1, @@ -3061,7 +3061,7 @@ cagg_rebuild_view_definition(ContinuousAgg *agg, Hypertable *mat_ht, bool force_ } else elog(DEBUG1, - "[cagg_rebuild_view_definition] %s.%s has being rebuilded!", + "[cagg_rebuild_view_definition] %s.%s has been rebuilt!", NameStr(agg->data.user_view_schema), NameStr(agg->data.user_view_name)); diff --git a/tsl/test/expected/cagg_repair.out b/tsl/test/expected/cagg_repair.out index 03e85ae3b68..91a14b0f6a9 100644 --- a/tsl/test/expected/cagg_repair.out +++ b/tsl/test/expected/cagg_repair.out @@ -118,7 +118,7 @@ SELECT * FROM conditions_summary ORDER BY bucket, device_name; (4 rows) CALL _timescaledb_internal.cagg_try_repair('conditions_summary', TRUE); -DEBUG: [cagg_rebuild_view_definition] public.conditions_summary has being rebuilded! +DEBUG: [cagg_rebuild_view_definition] public.conditions_summary has been rebuilt! \d+ conditions_summary View "public.conditions_summary" Column | Type | Collation | Nullable | Default | Storage | Description @@ -231,7 +231,7 @@ SELECT * FROM conditions_summary ORDER BY bucket, device_name; (7 rows) CALL _timescaledb_internal.cagg_try_repair('conditions_summary', TRUE); -DEBUG: [cagg_rebuild_view_definition] public.conditions_summary has being rebuilded! +DEBUG: [cagg_rebuild_view_definition] public.conditions_summary has been rebuilt! \d+ conditions_summary View "public.conditions_summary" Column | Type | Collation | Nullable | Default | Storage | Description @@ -311,4 +311,45 @@ UNION ALL WHERE conditions."time" >= COALESCE(_timescaledb_internal.to_timestamp(_timescaledb_internal.cagg_watermark(3)), '-infinity'::timestamp with time zone) GROUP BY (time_bucket('@ 7 days'::interval, conditions."time")); +-- Tests with old cagg format +CREATE MATERIALIZED VIEW conditions_summary_old_format +WITH (timescaledb.continuous, timescaledb.finalized=false) AS +SELECT + time_bucket(INTERVAL '1 week', "time") AS bucket, + MIN(temperature), + MAX(temperature), + SUM(temperature) +FROM + conditions +GROUP BY + 1 +WITH NO DATA; +-- Should rebuild without forcing +CALL _timescaledb_internal.cagg_try_repair('conditions_summary_old_format', FALSE); +DEBUG: [cagg_rebuild_view_definition] public.conditions_summary_old_format has been rebuilt! +\d+ conditions_summary_old_format + View "public.conditions_summary_old_format" + Column | Type | Collation | Nullable | Default | Storage | Description +--------+--------------------------+-----------+----------+---------+---------+------------- + bucket | timestamp with time zone | | | | plain | + min | integer | | | | plain | + max | integer | | | | plain | + sum | bigint | | | | plain | +View definition: + SELECT _materialized_hypertable_4.bucket, + _timescaledb_internal.finalize_agg('pg_catalog.min(integer)'::text, NULL::name, NULL::name, '{{pg_catalog,int4}}'::name[], _materialized_hypertable_4.agg_2_2, NULL::integer) AS min, + _timescaledb_internal.finalize_agg('pg_catalog.max(integer)'::text, NULL::name, NULL::name, '{{pg_catalog,int4}}'::name[], _materialized_hypertable_4.agg_3_3, NULL::integer) AS max, + _timescaledb_internal.finalize_agg('pg_catalog.sum(integer)'::text, NULL::name, NULL::name, '{{pg_catalog,int4}}'::name[], _materialized_hypertable_4.agg_4_4, NULL::bigint) AS sum + FROM _timescaledb_internal._materialized_hypertable_4 + WHERE _materialized_hypertable_4.bucket < COALESCE(_timescaledb_internal.to_timestamp(_timescaledb_internal.cagg_watermark(4)), '-infinity'::timestamp with time zone) + GROUP BY _materialized_hypertable_4.bucket +UNION ALL + SELECT time_bucket('@ 7 days'::interval, conditions."time") AS bucket, + min(conditions.temperature) AS min, + max(conditions.temperature) AS max, + sum(conditions.temperature) AS sum + FROM conditions + WHERE conditions."time" >= COALESCE(_timescaledb_internal.to_timestamp(_timescaledb_internal.cagg_watermark(4)), '-infinity'::timestamp with time zone) + GROUP BY (time_bucket('@ 7 days'::interval, conditions."time")); + DROP PROCEDURE _timescaledb_internal.cagg_try_repair (REGCLASS, BOOLEAN); diff --git a/tsl/test/sql/cagg_repair.sql b/tsl/test/sql/cagg_repair.sql index ee4743ac819..795e0129180 100644 --- a/tsl/test/sql/cagg_repair.sql +++ b/tsl/test/sql/cagg_repair.sql @@ -109,4 +109,22 @@ WITH NO DATA; CALL _timescaledb_internal.cagg_try_repair('conditions_summary_nojoin', TRUE); \d+ conditions_summary_nojoin +-- Tests with old cagg format +CREATE MATERIALIZED VIEW conditions_summary_old_format +WITH (timescaledb.continuous, timescaledb.finalized=false) AS +SELECT + time_bucket(INTERVAL '1 week', "time") AS bucket, + MIN(temperature), + MAX(temperature), + SUM(temperature) +FROM + conditions +GROUP BY + 1 +WITH NO DATA; + +-- Should rebuild without forcing +CALL _timescaledb_internal.cagg_try_repair('conditions_summary_old_format', FALSE); +\d+ conditions_summary_old_format + DROP PROCEDURE _timescaledb_internal.cagg_try_repair (REGCLASS, BOOLEAN);