From 23b24fff0e854c2319031f60c4c64870c4488373 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Wed, 12 Aug 2026 11:34:55 -0500 Subject: [PATCH v9 7/8] Fix VACUUM's handling of TOAST storage parameters. Per the documentation for CREATE TABLE: If a table parameter value is set and the equivalent toast. parameter is not, the TOAST table will use the table's parameter value. Presently, VACUUM does no such thing. It reads the TOAST table's own reloptions, which hold only what was set through toast.*, so vacuum_index_cleanup or vacuum_truncate set on the main table has no effect on its TOAST table. To fix, add merge_toast_reloptions(), which walks the parse table for StdRdOptions and takes the main table's value for anything the TOAST table left at its default. vacuum_rel() hands the main table's parameters down when recursing to a TOAST table, and it merges them into a copy of the TOAST table's parameters before the values are used. This doesn't help VACUUM against a TOAST table directly (e.g., "VACUUM pg_toast.pg_toast_5432"), but that's probably okay because it's not the main supported way to vacuum a TOAST table (see VACUUM's PROCESS_MAIN and PROCESS_TOAST options). A follow-up commit will do the same for autovacuum. While this is a bug fix, it's too intrusive for back-patching, but the issue seems to have gone unnoticed for a very long time, anyway. --- src/backend/access/common/reloptions.c | 93 +++++++++++++++++++ src/backend/commands/vacuum.c | 45 ++++++--- src/backend/postmaster/autovacuum.c | 1 + src/include/access/reloptions.h | 2 + src/include/commands/vacuum.h | 7 ++ .../injection_points/expected/vacuum.out | 11 +++ .../modules/injection_points/sql/vacuum.sql | 8 ++ 7 files changed, 156 insertions(+), 11 deletions(-) diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c index 4548eb02676..263ebf44a45 100644 --- a/src/backend/access/common/reloptions.c +++ b/src/backend/access/common/reloptions.c @@ -2114,6 +2114,99 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind) lengthof(stdRdOptionsTab)); } +/* + * find reloption + * Look up a reloption of the given kind by name. + * + * Returns NULL if no such option can be set on relations of that kind. Note + * that names are unique only within a kind; "fillfactor", for example, is + * declared separately for heaps and for each index access method. + */ +static relopt_gen * +find_reloption(const char *name, relopt_kind kind) +{ + if (need_initialization) + initialize_reloptions(); + + for (int i = 0; relOpts[i]; i++) + { + if ((relOpts[i]->kinds & kind) != 0 && + strcmp(relOpts[i]->name, name) == 0) + return relOpts[i]; + } + + return NULL; +} + +/* + * merge_toast_reloptions + * Fill in a TOAST table's unset options from its main table's. + * + * Any option that may be set on a TOAST table but was not is taken from + * main_opts. Either argument may be NULL; if both are, NULL is returned. + * Otherwise, the options to use are returned. + * + * An option counts as unset while it still holds the default declared for it + * above, which works because nothing a TOAST table accepts has a default the + * user could also set (see assert_toast_defaults_unsettable()). + * + * NB: This destructively modifies toast_opts, and what it returns may be + * either argument, so the caller must know which of the two it owns. + */ +StdRdOptions * +merge_toast_reloptions(StdRdOptions *toast_opts, StdRdOptions *main_opts) +{ + if (toast_opts == NULL) + return main_opts; + if (main_opts == NULL) + return toast_opts; + + for (int i = 0; i < lengthof(stdRdOptionsTab); i++) + { + const relopt_parse_elt *elem = &stdRdOptionsTab[i]; + relopt_gen *gen; + char *toast_val; + char *main_val; + + /* Skip anything that cannot be set on a TOAST table. */ + gen = find_reloption(elem->optname, RELOPT_KIND_TOAST); + if (gen == NULL) + continue; + + toast_val = (char *) toast_opts + elem->offset; + main_val = (char *) main_opts + elem->offset; + + switch (gen->type) + { + case RELOPT_TYPE_TERNARY: + if (*(pg_ternary *) toast_val == PG_TERNARY_UNSET) + *(pg_ternary *) toast_val = *(pg_ternary *) main_val; + break; + + case RELOPT_TYPE_INT: + if (*(int *) toast_val == ((relopt_int *) gen)->default_val) + *(int *) toast_val = *(int *) main_val; + break; + + case RELOPT_TYPE_REAL: + if (*(double *) toast_val == ((relopt_real *) gen)->default_val) + *(double *) toast_val = *(double *) main_val; + break; + + case RELOPT_TYPE_ENUM: + if (*(int *) toast_val == ((relopt_enum *) gen)->default_val) + *(int *) toast_val = *(int *) main_val; + break; + + default: + elog(ERROR, "reloption \"%s\" has a type a TOAST table cannot inherit", + elem->optname); + } + } + + return toast_opts; +} + /* * build_reloptions * diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 52116c02b59..8fe48920c9f 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -31,6 +31,7 @@ #include "access/heapam.h" #include "access/htup_details.h" #include "access/multixact.h" +#include "access/reloptions.h" #include "access/tableam.h" #include "access/transam.h" #include "access/xact.h" @@ -187,6 +188,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel) /* Will be set later if we recurse to a TOAST table. */ params.toast_parent = InvalidOid; + params.main_relopts = NULL; /* * Set this to an invalid value so it is clear whether or not a @@ -2039,6 +2041,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, int save_sec_context; int save_nestlevel; VacuumParams toast_vacuum_params; + StdRdOptions *relopts; + StdRdOptions relopts_copy; /* * This function scribbles on the parameters, so make a copy early to @@ -2201,6 +2205,19 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, lockrelid = rel->rd_lockInfo.lockRelId; LockRelationIdForSession(&lockrelid, lmode); + /* + * A TOAST table takes any storage parameter it accepts but does not set + * from its main table, whose parameters the caller handed down for that + * purpose. Merge them into a copy of our own. + * + * We are careful to avoid scribbling on the relcache's copy of the + * options. + */ + if (rel->rd_options) + memcpy(&relopts_copy, rel->rd_options, sizeof(StdRdOptions)); + relopts = merge_toast_reloptions(rel->rd_options ? &relopts_copy : NULL, + params.main_relopts); + /* * Set index_cleanup option based on index_cleanup reloption if it wasn't * specified in VACUUM command, or when running in an autovacuum worker @@ -2209,11 +2226,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, { StdRdOptIndexCleanup vacuum_index_cleanup; - if (rel->rd_options == NULL) + if (relopts == NULL) vacuum_index_cleanup = STDRD_OPTION_VACUUM_INDEX_CLEANUP_NOT_SET; else - vacuum_index_cleanup = - ((StdRdOptions *) rel->rd_options)->vacuum_index_cleanup; + vacuum_index_cleanup = relopts->vacuum_index_cleanup; switch (vacuum_index_cleanup) { @@ -2245,10 +2261,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, * Check if the vacuum_max_eager_freeze_failure_rate table storage * parameter was specified. This overrides the GUC value. */ - if (rel->rd_options != NULL && - ((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate >= 0) - params.max_eager_freeze_failure_rate = - ((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate; + if (relopts != NULL && relopts->vacuum_max_eager_freeze_failure_rate >= 0) + params.max_eager_freeze_failure_rate = relopts->vacuum_max_eager_freeze_failure_rate; /* * Set truncate option based on truncate reloption or GUC if it wasn't @@ -2256,11 +2270,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, */ if (params.truncate == VACOPTVALUE_UNSPECIFIED) { - StdRdOptions *opts = (StdRdOptions *) rel->rd_options; - - if (opts && opts->vacuum_truncate != PG_TERNARY_UNSET) + if (relopts && relopts->vacuum_truncate != PG_TERNARY_UNSET) { - if (opts->vacuum_truncate == PG_TERNARY_TRUE) + if (relopts->vacuum_truncate == PG_TERNARY_TRUE) params.truncate = VACOPTVALUE_ENABLED; else params.truncate = VACOPTVALUE_DISABLED; @@ -2293,6 +2305,17 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params, else toast_relid = InvalidOid; + /* + * Hand our storage parameters down for the TOAST table to inherit. Take + * a copy while we still have the relation open; the relcache entry can go + * away once we close it. + */ + if (OidIsValid(toast_relid) && rel->rd_options) + { + memcpy(&relopts_copy, rel->rd_options, sizeof(StdRdOptions)); + toast_vacuum_params.main_relopts = &relopts_copy; + } + /* * Switch to the table owner's userid, so that any index functions are run * as that user. Also lock down security-restricted operations and diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index a99f7108636..1610c60ec4b 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2891,6 +2891,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_params.log_vacuum_min_duration = log_vacuum_min_duration; tab->at_params.log_analyze_min_duration = log_analyze_min_duration; tab->at_params.toast_parent = InvalidOid; + tab->at_params.main_relopts = NULL; /* Determine the number of parallel vacuum workers to use */ tab->at_params.nworkers = 0; diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h index e8cb7f7a627..6c599382f02 100644 --- a/src/include/access/reloptions.h +++ b/src/include/access/reloptions.h @@ -247,6 +247,8 @@ extern void *build_local_reloptions(local_relopts *relopts, Datum options, extern bytea *default_reloptions(Datum reloptions, bool validate, relopt_kind kind); +extern struct StdRdOptions *merge_toast_reloptions(struct StdRdOptions *toast_opts, + struct StdRdOptions *main_opts); extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate); extern bytea *view_reloptions(Datum reloptions, bool validate); extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate); diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index e62f23748dc..2ebdd5e40ab 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -248,6 +248,13 @@ typedef struct VacuumParams * disabled. */ int nworkers; + + /* + * When vacuuming a TOAST table, its main table's storage parameters, for + * the TOAST table to inherit anything it doesn't set itself. NULL if the + * main table has none, or if this isn't a TOAST table. + */ + struct StdRdOptions *main_relopts; } VacuumParams; /* diff --git a/src/test/modules/injection_points/expected/vacuum.out b/src/test/modules/injection_points/expected/vacuum.out index 58df59fa927..caf0cc232b4 100644 --- a/src/test/modules/injection_points/expected/vacuum.out +++ b/src/test/modules/injection_points/expected/vacuum.out @@ -79,9 +79,20 @@ NOTICE: notice triggered for injection point vacuum-truncate-enabled NOTICE: notice triggered for injection point vacuum-index-cleanup-auto NOTICE: notice triggered for injection point vacuum-truncate-enabled RESET vacuum_truncate; +-- A TOAST table inherits what it does not set from its main table. +CREATE TABLE vac_tab_toast_inherit(i int, j text) WITH + (autovacuum_enabled=false, + vacuum_index_cleanup=false, + vacuum_truncate=false, toast.vacuum_truncate=true); +VACUUM vac_tab_toast_inherit; +NOTICE: notice triggered for injection point vacuum-index-cleanup-disabled +NOTICE: notice triggered for injection point vacuum-truncate-disabled +NOTICE: notice triggered for injection point vacuum-index-cleanup-disabled +NOTICE: notice triggered for injection point vacuum-truncate-enabled DROP TABLE vac_tab_auto; DROP TABLE vac_tab_on_toast_off; DROP TABLE vac_tab_off_toast_on; +DROP TABLE vac_tab_toast_inherit; -- Cleanup SELECT injection_points_detach('vacuum-index-cleanup-auto'); injection_points_detach diff --git a/src/test/modules/injection_points/sql/vacuum.sql b/src/test/modules/injection_points/sql/vacuum.sql index 23760dd0f38..0a43e14c928 100644 --- a/src/test/modules/injection_points/sql/vacuum.sql +++ b/src/test/modules/injection_points/sql/vacuum.sql @@ -33,9 +33,17 @@ SET vacuum_truncate = true; VACUUM vac_tab_auto; RESET vacuum_truncate; +-- A TOAST table inherits what it does not set from its main table. +CREATE TABLE vac_tab_toast_inherit(i int, j text) WITH + (autovacuum_enabled=false, + vacuum_index_cleanup=false, + vacuum_truncate=false, toast.vacuum_truncate=true); +VACUUM vac_tab_toast_inherit; + DROP TABLE vac_tab_auto; DROP TABLE vac_tab_on_toast_off; DROP TABLE vac_tab_off_toast_on; +DROP TABLE vac_tab_toast_inherit; -- Cleanup SELECT injection_points_detach('vacuum-index-cleanup-auto'); -- 2.50.1 (Apple Git-155)