Skip to content

Commit

Permalink
Fix cagg_repair for the old CAgg format
Browse files Browse the repository at this point in the history
In commit 4a6650d 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 3c8d7ce)
  • Loading branch information
fabriziomello authored and timescale-automation committed Apr 26, 2023
1 parent 4fc2cab commit 676b5a7
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 4 deletions.
4 changes: 2 additions & 2 deletions tsl/src/continuous_aggs/create.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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));

Expand Down
45 changes: 43 additions & 2 deletions tsl/test/expected/cagg_repair.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
18 changes: 18 additions & 0 deletions tsl/test/sql/cagg_repair.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 676b5a7

Please sign in to comment.