diff --git a/.unreleased/pr_9024 b/.unreleased/pr_9024 new file mode 100644 index 00000000000..8e59be3af3f --- /dev/null +++ b/.unreleased/pr_9024 @@ -0,0 +1 @@ +Fixes: #9024 Recompress some chunks on VACUUM FULL diff --git a/src/process_utility.c b/src/process_utility.c index e9d00aa8846..9273562bac7 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -987,10 +987,64 @@ foreach_chunk_multitransaction(Oid relid, MemoryContext mctx, mt_process_chunk_t typedef struct VacuumCtx { + bool is_vacuumfull; VacuumRelation *ht_vacuum_rel; List *chunk_rels; + List *recompress_chunk_oids; } VacuumCtx; +static bool +chunk_has_missing_attrs(Chunk *chunk) +{ + bool has_missing_attrs = false; + Relation ht_rel = relation_open(chunk->hypertable_relid, AccessShareLock); + Relation chunk_rel = relation_open(chunk->table_id, AccessShareLock); + TupleDesc tupdesc = RelationGetDescr(chunk_rel); + + for (int i = 0; i < tupdesc->natts; i++) + { + Form_pg_attribute att = TupleDescAttr(tupdesc, i); + + if (att->atthasmissing) + { + has_missing_attrs = true; + break; + } + } + + relation_close(chunk_rel, AccessShareLock); + relation_close(ht_rel, AccessShareLock); + return has_missing_attrs; +} + +static void +check_chunk_needs_recompression(Oid chunk_relid, VacuumCtx *ctx) +{ + /* Only VACUUM FULL does a complete table rewrite */ + if (!ctx->is_vacuumfull) + return; + + Chunk *chunk = ts_chunk_get_by_relid(chunk_relid, false); + if (!chunk) + return; + + /* + * VACUUM FULL will materialize missing attributes. Recompress chunks to propagate + * these changes to compressed data. + */ + if (chunk_has_missing_attrs(chunk)) + { + /* + * Frozen chunks are not allowed to add columns with defaults + */ + Ensure(!ts_chunk_is_frozen(chunk), + "chunk \"%s.%s\" was altered unsafely after it was frozen", + NameStr(chunk->fd.schema_name), + NameStr(chunk->fd.table_name)); + ctx->recompress_chunk_oids = lappend_oid(ctx->recompress_chunk_oids, chunk_relid); + } +} + /* Adds a chunk to the list of tables to be vacuumed */ static void add_chunk_to_vacuum(Hypertable *ht, Oid chunk_relid, void *arg) @@ -1018,6 +1072,8 @@ add_chunk_to_vacuum(Hypertable *ht, Oid chunk_relid, void *arg) ctx->chunk_rels = lappend(ctx->chunk_rels, chunk_vacuum_rel); } } + + check_chunk_needs_recompression(chunk_relid, ctx); } /* @@ -1026,7 +1082,7 @@ add_chunk_to_vacuum(Hypertable *ht, Oid chunk_relid, void *arg) * from vacuum.c. */ static List * -ts_get_all_vacuum_rels(bool is_vacuumcmd) +ts_get_all_vacuum_rels(bool is_vacuumcmd, VacuumCtx *ctx) { List *vacrels = NIL; Relation pgclass; @@ -1066,6 +1122,8 @@ ts_get_all_vacuum_rels(bool is_vacuumcmd) * about failure to open one of these relations later. */ vacrels = lappend(vacrels, makeVacuumRelation(NULL, relid, NIL)); + + check_chunk_needs_recompression(relid, ctx); } table_endscan(scan); @@ -1082,8 +1140,10 @@ process_vacuum(ProcessUtilityArgs *args) VacuumStmt *stmt = (VacuumStmt *) args->parsetree; bool is_toplevel = (args->context == PROCESS_UTILITY_TOPLEVEL); VacuumCtx ctx = { + .is_vacuumfull = false, .ht_vacuum_rel = NULL, .chunk_rels = NIL, + .recompress_chunk_oids = NIL, }; ListCell *lc; Hypertable *ht; @@ -1094,24 +1154,22 @@ process_vacuum(ProcessUtilityArgs *args) is_vacuumcmd = stmt->is_vacuumcmd; -#if PG16_GE - if (is_vacuumcmd) + /* Look for new option ONLY_DATABASE_STATS and FULL */ + foreach (lc, stmt->options) { - /* Look for new option ONLY_DATABASE_STATS */ - foreach (lc, stmt->options) - { - DefElem *opt = (DefElem *) lfirst(lc); - - /* if "only_database_stats" is defined then don't execute our custom code and return to - * the postgres execution for the proper validations */ - if (strcmp(opt->defname, "only_database_stats") == 0) - return DDL_CONTINUE; - } - } + DefElem *opt = (DefElem *) lfirst(lc); +#if PG16_GE + /* if "only_database_stats" is defined then don't execute our custom code and return to + * the postgres execution for the proper validations */ + if (is_vacuumcmd && strcmp(opt->defname, "only_database_stats") == 0) + return DDL_CONTINUE; #endif + if (strcmp(opt->defname, "full") == 0) + ctx.is_vacuumfull = defGetBoolean(opt); + } if (stmt->rels == NIL) - vacuum_rels = ts_get_all_vacuum_rels(is_vacuumcmd); + vacuum_rels = ts_get_all_vacuum_rels(is_vacuumcmd, &ctx); else { Cache *hcache = ts_hypertable_cache_pin(); @@ -1147,6 +1205,20 @@ process_vacuum(ProcessUtilityArgs *args) { PreventCommandDuringRecovery(is_vacuumcmd ? "VACUUM" : "ANALYZE"); + if (list_length(ctx.recompress_chunk_oids) > 0) + { + /* there are chunks that need recompression */ + ListCell *lc; + foreach (lc, ctx.recompress_chunk_oids) + { + /* TODO: needs to handle partial compression */ + Oid chunk_relid = lfirst_oid(lc); + (void) DirectFunctionCall3(ts_cm_functions->compress_chunk, + ObjectIdGetDatum(chunk_relid), + BoolGetDatum(true), + BoolGetDatum(true)); /* recompress */ + } + } /* ACL permission checks inside vacuum_rel and analyze_rel called by this ExecVacuum */ ExecVacuum(args->parse_state, stmt, is_toplevel); } diff --git a/tsl/test/expected/vacuum.out b/tsl/test/expected/vacuum.out new file mode 100644 index 00000000000..6c60af4b793 --- /dev/null +++ b/tsl/test/expected/vacuum.out @@ -0,0 +1,47 @@ +-- This file and its contents are licensed under the Timescale License. +-- Please see the included NOTICE for copyright information and +-- LICENSE-TIMESCALE for a copy of the license. +\c :TEST_DBNAME :ROLE_SUPERUSER +-- Test VACUUM FULL with compressed chunks and missing attributes +CREATE TABLE vacuum_missing_test(ts int, c1 int); +SELECT create_hypertable('vacuum_missing_test', 'ts', chunk_time_interval => 1000); + create_hypertable +---------------------------------- + (1,public,vacuum_missing_test,t) + +INSERT INTO vacuum_missing_test VALUES (0, 1); +ALTER TABLE vacuum_missing_test SET (timescaledb.compress, timescaledb.compress_segmentby = ''); +SELECT compress_chunk(show_chunks('vacuum_missing_test'), true); + compress_chunk +---------------------------------------- + _timescaledb_internal._hyper_1_1_chunk + +ALTER TABLE vacuum_missing_test ADD COLUMN c2 int DEFAULT 7; +VACUUM FULL ANALYZE vacuum_missing_test; +SELECT * FROM vacuum_missing_test; + ts | c1 | c2 +----+----+---- + 0 | 1 | 7 + +DROP TABLE vacuum_missing_test; +CREATE TABLE vacuum_missing_test(ts int, c1 int); +SELECT create_hypertable('vacuum_missing_test', 'ts', chunk_time_interval => 1000); + create_hypertable +---------------------------------- + (3,public,vacuum_missing_test,t) + +INSERT INTO vacuum_missing_test VALUES (0, 1); +ALTER TABLE vacuum_missing_test SET (timescaledb.compress, timescaledb.compress_segmentby = ''); +SELECT compress_chunk(show_chunks('vacuum_missing_test'), true); + compress_chunk +---------------------------------------- + _timescaledb_internal._hyper_3_4_chunk + +ALTER TABLE vacuum_missing_test ADD COLUMN c2 int DEFAULT 7; +VACUUM FULL; +SELECT * FROM vacuum_missing_test; + ts | c1 | c2 +----+----+---- + 0 | 1 | 7 + +DROP TABLE vacuum_missing_test; diff --git a/tsl/test/sql/CMakeLists.txt b/tsl/test/sql/CMakeLists.txt index 9b3df2e7384..d5c01876e78 100644 --- a/tsl/test/sql/CMakeLists.txt +++ b/tsl/test/sql/CMakeLists.txt @@ -166,7 +166,8 @@ if(CMAKE_BUILD_TYPE MATCHES Debug) vector_agg_text.sql vector_agg_memory.sql vector_agg_segmentby.sql - vector_agg_uuid.sql) + vector_agg_uuid.sql + vacuum.sql) list( APPEND diff --git a/tsl/test/sql/vacuum.sql b/tsl/test/sql/vacuum.sql new file mode 100644 index 00000000000..9c699aafd85 --- /dev/null +++ b/tsl/test/sql/vacuum.sql @@ -0,0 +1,26 @@ +-- This file and its contents are licensed under the Timescale License. +-- Please see the included NOTICE for copyright information and +-- LICENSE-TIMESCALE for a copy of the license. + +\c :TEST_DBNAME :ROLE_SUPERUSER + +-- Test VACUUM FULL with compressed chunks and missing attributes +CREATE TABLE vacuum_missing_test(ts int, c1 int); +SELECT create_hypertable('vacuum_missing_test', 'ts', chunk_time_interval => 1000); +INSERT INTO vacuum_missing_test VALUES (0, 1); +ALTER TABLE vacuum_missing_test SET (timescaledb.compress, timescaledb.compress_segmentby = ''); +SELECT compress_chunk(show_chunks('vacuum_missing_test'), true); +ALTER TABLE vacuum_missing_test ADD COLUMN c2 int DEFAULT 7; +VACUUM FULL ANALYZE vacuum_missing_test; +SELECT * FROM vacuum_missing_test; +DROP TABLE vacuum_missing_test; + +CREATE TABLE vacuum_missing_test(ts int, c1 int); +SELECT create_hypertable('vacuum_missing_test', 'ts', chunk_time_interval => 1000); +INSERT INTO vacuum_missing_test VALUES (0, 1); +ALTER TABLE vacuum_missing_test SET (timescaledb.compress, timescaledb.compress_segmentby = ''); +SELECT compress_chunk(show_chunks('vacuum_missing_test'), true); +ALTER TABLE vacuum_missing_test ADD COLUMN c2 int DEFAULT 7; +VACUUM FULL; +SELECT * FROM vacuum_missing_test; +DROP TABLE vacuum_missing_test;