On Tue, Jun 24, 2025 at 11:30:13AM -0500, Nathan Bossart wrote: > On Tue, Jun 24, 2025 at 10:18:18AM +0900, Michael Paquier wrote: > > Knowing that I'm indirectly responsible for this mess, I would like to > > take care of that myself. Would that be OK for you? > > I would be grateful if you took care of it.
Okay, applied the main fix down to v13 then. > +1 for this. I did something similar to verify the other bugs I reported, > and this seems far easier to maintain than potentially-unstable tests that > require lots of setup and that depend on secondary effects. After sleeping on this point, I have finished with this idea for tests down to v17, adding more coverage for v18 with the GUC vacuum_truncate and the "auto" mode of index_cleanup, while on it. v16 and older branches don't have tests, but I have checked the sanity of the two code paths using the tests attached for vacuum.sql and pgstattuple.sql, at least. Two truncation tests for the "false" cases were actually unstable. Sometimes the truncation may not happen even with an aggressive VACUUM. Anyway, it is enough to check the true case for the two code paths patched, even if it may not happen at 100%. I did not notice a problem with the index_cleanup case as long as we had enough pages in the TOAST btree index, requiring some runtime to load the entries, so the cost was annoying. Attached is the remaining patch for HEAD, planned once v19 opens, and the tests I have used on the back-branches as a txt to not confuse the CI, for reference. -- Michael
From 32b561d88e7480cd0ab9f79443fbacd4f31dc53c Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 25 Jun 2025 10:22:08 +0900
Subject: [PATCH v5] Refactor handling of VacuumParams
This adds a couple of const markers, while removing pointer uses of
VacuumParams to force the following policy among all the VACUUM-related
calls: do not touch the values given by the caller.
This patch is for HEAD.
Author: Michael P.
---
src/include/access/heapam.h | 4 +-
src/include/access/tableam.h | 6 +-
src/include/commands/vacuum.h | 6 +-
src/backend/access/heap/vacuumlazy.c | 44 +++++-----
src/backend/commands/analyze.c | 26 +++---
src/backend/commands/cluster.c | 2 +-
src/backend/commands/vacuum.c | 118 ++++++++++++---------------
src/backend/postmaster/autovacuum.c | 2 +-
8 files changed, 98 insertions(+), 110 deletions(-)
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 3a9424c19c9a..a2bd5a897f87 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -21,6 +21,7 @@
#include "access/skey.h"
#include "access/table.h" /* for backward compatibility */
#include "access/tableam.h"
+#include "commands/vacuum.h"
#include "nodes/lockoptions.h"
#include "nodes/primnodes.h"
#include "storage/bufpage.h"
@@ -396,9 +397,8 @@ extern void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
OffsetNumber *unused, int nunused);
/* in heap/vacuumlazy.c */
-struct VacuumParams;
extern void heap_vacuum_rel(Relation rel,
- struct VacuumParams *params, BufferAccessStrategy bstrategy);
+ const VacuumParams params, BufferAccessStrategy bstrategy);
/* in heap/heapam_visibility.c */
extern bool HeapTupleSatisfiesVisibility(HeapTuple htup, Snapshot snapshot,
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 8713e12cbfb9..1c9e802a6b12 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -20,6 +20,7 @@
#include "access/relscan.h"
#include "access/sdir.h"
#include "access/xact.h"
+#include "commands/vacuum.h"
#include "executor/tuptable.h"
#include "storage/read_stream.h"
#include "utils/rel.h"
@@ -36,7 +37,6 @@ extern PGDLLIMPORT bool synchronize_seqscans;
struct BulkInsertStateData;
struct IndexInfo;
struct SampleScanState;
-struct VacuumParams;
struct ValidateIndexState;
/*
@@ -645,7 +645,7 @@ typedef struct TableAmRoutine
* integrate with autovacuum's scheduling.
*/
void (*relation_vacuum) (Relation rel,
- struct VacuumParams *params,
+ const VacuumParams params,
BufferAccessStrategy bstrategy);
/*
@@ -1664,7 +1664,7 @@ table_relation_copy_for_cluster(Relation OldTable, Relation NewTable,
* routine, even if (for ANALYZE) it is part of the same VACUUM command.
*/
static inline void
-table_relation_vacuum(Relation rel, struct VacuumParams *params,
+table_relation_vacuum(Relation rel, const VacuumParams params,
BufferAccessStrategy bstrategy)
{
rel->rd_tableam->relation_vacuum(rel, params, bstrategy);
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index bc37a80dc74f..14eeccbd7185 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -336,7 +336,7 @@ extern PGDLLIMPORT int64 parallel_vacuum_worker_delay_ns;
/* in commands/vacuum.c */
extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel);
-extern void vacuum(List *relations, VacuumParams *params,
+extern void vacuum(List *relations, const VacuumParams params,
BufferAccessStrategy bstrategy, MemoryContext vac_context,
bool isTopLevel);
extern void vac_open_indexes(Relation relation, LOCKMODE lockmode,
@@ -357,7 +357,7 @@ extern void vac_update_relstats(Relation relation,
bool *frozenxid_updated,
bool *minmulti_updated,
bool in_outer_xact);
-extern bool vacuum_get_cutoffs(Relation rel, const VacuumParams *params,
+extern bool vacuum_get_cutoffs(Relation rel, const VacuumParams params,
struct VacuumCutoffs *cutoffs);
extern bool vacuum_xid_failsafe_check(const struct VacuumCutoffs *cutoffs);
extern void vac_update_datfrozenxid(void);
@@ -398,7 +398,7 @@ extern void parallel_vacuum_main(dsm_segment *seg, shm_toc *toc);
/* in commands/analyze.c */
extern void analyze_rel(Oid relid, RangeVar *relation,
- VacuumParams *params, List *va_cols, bool in_outer_xact,
+ const VacuumParams params, List *va_cols, bool in_outer_xact,
BufferAccessStrategy bstrategy);
extern bool std_typanalyze(VacAttrStats *stats);
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 09416450af96..56d456736da0 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -423,7 +423,7 @@ typedef struct LVSavedErrInfo
/* non-export function prototypes */
static void lazy_scan_heap(LVRelState *vacrel);
static void heap_vacuum_eager_scan_setup(LVRelState *vacrel,
- VacuumParams *params);
+ const VacuumParams params);
static BlockNumber heap_vac_scan_next_block(ReadStream *stream,
void *callback_private_data,
void *per_buffer_data);
@@ -485,7 +485,7 @@ static void restore_vacuum_error_info(LVRelState *vacrel,
* vacuum options or for relfrozenxid/relminmxid advancement.
*/
static void
-heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
+heap_vacuum_eager_scan_setup(LVRelState *vacrel, const VacuumParams params)
{
uint32 randseed;
BlockNumber allvisible;
@@ -504,7 +504,7 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
vacrel->eager_scan_remaining_successes = 0;
/* If eager scanning is explicitly disabled, just return. */
- if (params->max_eager_freeze_failure_rate == 0)
+ if (params.max_eager_freeze_failure_rate == 0)
return;
/*
@@ -581,11 +581,11 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
vacrel->next_eager_scan_region_start = randseed % EAGER_SCAN_REGION_SIZE;
- Assert(params->max_eager_freeze_failure_rate > 0 &&
- params->max_eager_freeze_failure_rate <= 1);
+ Assert(params.max_eager_freeze_failure_rate > 0 &&
+ params.max_eager_freeze_failure_rate <= 1);
vacrel->eager_scan_max_fails_per_region =
- params->max_eager_freeze_failure_rate *
+ params.max_eager_freeze_failure_rate *
EAGER_SCAN_REGION_SIZE;
/*
@@ -612,7 +612,7 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
* and locked the relation.
*/
void
-heap_vacuum_rel(Relation rel, VacuumParams *params,
+heap_vacuum_rel(Relation rel, const VacuumParams params,
BufferAccessStrategy bstrategy)
{
LVRelState *vacrel;
@@ -634,9 +634,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
ErrorContextCallback errcallback;
char **indnames = NULL;
- verbose = (params->options & VACOPT_VERBOSE) != 0;
+ verbose = (params.options & VACOPT_VERBOSE) != 0;
instrument = (verbose || (AmAutoVacuumWorkerProcess() &&
- params->log_min_duration >= 0));
+ params.log_min_duration >= 0));
if (instrument)
{
pg_rusage_init(&ru0);
@@ -699,9 +699,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* The truncate param allows user to avoid attempting relation truncation,
* though it can't force truncation to happen.
*/
- Assert(params->index_cleanup != VACOPTVALUE_UNSPECIFIED);
- Assert(params->truncate != VACOPTVALUE_UNSPECIFIED &&
- params->truncate != VACOPTVALUE_AUTO);
+ Assert(params.index_cleanup != VACOPTVALUE_UNSPECIFIED);
+ Assert(params.truncate != VACOPTVALUE_UNSPECIFIED &&
+ params.truncate != VACOPTVALUE_AUTO);
/*
* While VacuumFailSafeActive is reset to false before calling this, we
@@ -711,14 +711,14 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
vacrel->consider_bypass_optimization = true;
vacrel->do_index_vacuuming = true;
vacrel->do_index_cleanup = true;
- vacrel->do_rel_truncate = (params->truncate != VACOPTVALUE_DISABLED);
- if (params->index_cleanup == VACOPTVALUE_DISABLED)
+ vacrel->do_rel_truncate = (params.truncate != VACOPTVALUE_DISABLED);
+ if (params.index_cleanup == VACOPTVALUE_DISABLED)
{
/* Force disable index vacuuming up-front */
vacrel->do_index_vacuuming = false;
vacrel->do_index_cleanup = false;
}
- else if (params->index_cleanup == VACOPTVALUE_ENABLED)
+ else if (params.index_cleanup == VACOPTVALUE_ENABLED)
{
/* Force index vacuuming. Note that failsafe can still bypass. */
vacrel->consider_bypass_optimization = false;
@@ -726,7 +726,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
else
{
/* Default/auto, make all decisions dynamically */
- Assert(params->index_cleanup == VACOPTVALUE_AUTO);
+ Assert(params.index_cleanup == VACOPTVALUE_AUTO);
}
/* Initialize page counters explicitly (be tidy) */
@@ -789,7 +789,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
*/
vacrel->skippedallvis = false;
skipwithvm = true;
- if (params->options & VACOPT_DISABLE_PAGE_SKIPPING)
+ if (params.options & VACOPT_DISABLE_PAGE_SKIPPING)
{
/*
* Force aggressive mode, and disable skipping blocks using the
@@ -830,7 +830,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* is already dangerously old.)
*/
lazy_check_wraparound_failsafe(vacrel);
- dead_items_alloc(vacrel, params->nworkers);
+ dead_items_alloc(vacrel, params.nworkers);
/*
* Call lazy_scan_heap to perform all required heap pruning, index
@@ -947,9 +947,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
{
TimestampTz endtime = GetCurrentTimestamp();
- if (verbose || params->log_min_duration == 0 ||
+ if (verbose || params.log_min_duration == 0 ||
TimestampDifferenceExceeds(starttime, endtime,
- params->log_min_duration))
+ params.log_min_duration))
{
long secs_dur;
int usecs_dur;
@@ -984,10 +984,10 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
* Aggressiveness already reported earlier, in dedicated
* VACUUM VERBOSE ereport
*/
- Assert(!params->is_wraparound);
+ Assert(!params.is_wraparound);
msgfmt = _("finished vacuuming \"%s.%s.%s\": index scans: %d\n");
}
- else if (params->is_wraparound)
+ else if (params.is_wraparound)
{
/*
* While it's possible for a VACUUM to be both is_wraparound
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 4fffb76e5573..7111d5d5334f 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -76,7 +76,7 @@ static BufferAccessStrategy vac_strategy;
static void do_analyze_rel(Relation onerel,
- VacuumParams *params, List *va_cols,
+ const VacuumParams params, List *va_cols,
AcquireSampleRowsFunc acquirefunc, BlockNumber relpages,
bool inh, bool in_outer_xact, int elevel);
static void compute_index_stats(Relation onerel, double totalrows,
@@ -107,7 +107,7 @@ static Datum ind_fetch_func(VacAttrStatsP stats, int rownum, bool *isNull);
*/
void
analyze_rel(Oid relid, RangeVar *relation,
- VacuumParams *params, List *va_cols, bool in_outer_xact,
+ const VacuumParams params, List *va_cols, bool in_outer_xact,
BufferAccessStrategy bstrategy)
{
Relation onerel;
@@ -116,7 +116,7 @@ analyze_rel(Oid relid, RangeVar *relation,
BlockNumber relpages = 0;
/* Select logging level */
- if (params->options & VACOPT_VERBOSE)
+ if (params.options & VACOPT_VERBOSE)
elevel = INFO;
else
elevel = DEBUG2;
@@ -138,8 +138,8 @@ analyze_rel(Oid relid, RangeVar *relation,
*
* Make sure to generate only logs for ANALYZE in this case.
*/
- onerel = vacuum_open_relation(relid, relation, params->options & ~(VACOPT_VACUUM),
- params->log_min_duration >= 0,
+ onerel = vacuum_open_relation(relid, relation, params.options & ~(VACOPT_VACUUM),
+ params.log_min_duration >= 0,
ShareUpdateExclusiveLock);
/* leave if relation could not be opened or locked */
@@ -155,7 +155,7 @@ analyze_rel(Oid relid, RangeVar *relation,
*/
if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
onerel->rd_rel,
- params->options & ~VACOPT_VACUUM))
+ params.options & ~VACOPT_VACUUM))
{
relation_close(onerel, ShareUpdateExclusiveLock);
return;
@@ -227,7 +227,7 @@ analyze_rel(Oid relid, RangeVar *relation,
else
{
/* No need for a WARNING if we already complained during VACUUM */
- if (!(params->options & VACOPT_VACUUM))
+ if (!(params.options & VACOPT_VACUUM))
ereport(WARNING,
(errmsg("skipping \"%s\" --- cannot analyze non-tables or special system tables",
RelationGetRelationName(onerel))));
@@ -275,7 +275,7 @@ analyze_rel(Oid relid, RangeVar *relation,
* appropriate acquirefunc for each child table.
*/
static void
-do_analyze_rel(Relation onerel, VacuumParams *params,
+do_analyze_rel(Relation onerel, const VacuumParams params,
List *va_cols, AcquireSampleRowsFunc acquirefunc,
BlockNumber relpages, bool inh, bool in_outer_xact,
int elevel)
@@ -309,9 +309,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
PgStat_Counter startreadtime = 0;
PgStat_Counter startwritetime = 0;
- verbose = (params->options & VACOPT_VERBOSE) != 0;
+ verbose = (params.options & VACOPT_VERBOSE) != 0;
instrument = (verbose || (AmAutoVacuumWorkerProcess() &&
- params->log_min_duration >= 0));
+ params.log_min_duration >= 0));
if (inh)
ereport(elevel,
(errmsg("analyzing \"%s.%s\" inheritance tree",
@@ -706,7 +706,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
* amvacuumcleanup() when called in ANALYZE-only mode. The only exception
* among core index AMs is GIN/ginvacuumcleanup().
*/
- if (!(params->options & VACOPT_VACUUM))
+ if (!(params.options & VACOPT_VACUUM))
{
for (ind = 0; ind < nindexes; ind++)
{
@@ -736,9 +736,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
{
TimestampTz endtime = GetCurrentTimestamp();
- if (verbose || params->log_min_duration == 0 ||
+ if (verbose || params.log_min_duration == 0 ||
TimestampDifferenceExceeds(starttime, endtime,
- params->log_min_duration))
+ params.log_min_duration))
{
long delay_in_ms;
WalUsage walusage;
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 54a08e4102e1..b55221d44cd0 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -917,7 +917,7 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex, bool verb
* not to be aggressive about this.
*/
memset(¶ms, 0, sizeof(VacuumParams));
- vacuum_get_cutoffs(OldHeap, ¶ms, &cutoffs);
+ vacuum_get_cutoffs(OldHeap, params, &cutoffs);
/*
* FreezeXid will become the table's new relfrozenxid, and that mustn't go
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 02993d320daf..733ef40ae7c5 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -124,7 +124,7 @@ static void vac_truncate_clog(TransactionId frozenXID,
MultiXactId minMulti,
TransactionId lastSaneFrozenXid,
MultiXactId lastSaneMinMulti);
-static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
+static bool vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
BufferAccessStrategy bstrategy);
static double compute_parallel_delay(void);
static VacOptValue get_vacoptval_from_boolean(DefElem *def);
@@ -465,7 +465,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
}
/* Now go through the common routine */
- vacuum(vacstmt->rels, ¶ms, bstrategy, vac_context, isTopLevel);
+ vacuum(vacstmt->rels, params, bstrategy, vac_context, isTopLevel);
/* Finally, clean up the vacuum memory context */
MemoryContextDelete(vac_context);
@@ -494,7 +494,7 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
* memory context that will not disappear at transaction commit.
*/
void
-vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
+vacuum(List *relations, const VacuumParams params, BufferAccessStrategy bstrategy,
MemoryContext vac_context, bool isTopLevel)
{
static bool in_vacuum = false;
@@ -503,9 +503,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
volatile bool in_outer_xact,
use_own_xacts;
- Assert(params != NULL);
-
- stmttype = (params->options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE";
+ stmttype = (params.options & VACOPT_VACUUM) ? "VACUUM" : "ANALYZE";
/*
* We cannot run VACUUM inside a user transaction block; if we were inside
@@ -515,7 +513,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
*
* ANALYZE (without VACUUM) can run either way.
*/
- if (params->options & VACOPT_VACUUM)
+ if (params.options & VACOPT_VACUUM)
{
PreventInTransactionBlock(isTopLevel, stmttype);
in_outer_xact = false;
@@ -538,7 +536,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
* Build list of relation(s) to process, putting any new data in
* vac_context for safekeeping.
*/
- if (params->options & VACOPT_ONLY_DATABASE_STATS)
+ if (params.options & VACOPT_ONLY_DATABASE_STATS)
{
/* We don't process any tables in this case */
Assert(relations == NIL);
@@ -554,7 +552,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
List *sublist;
MemoryContext old_context;
- sublist = expand_vacuum_rel(vrel, vac_context, params->options);
+ sublist = expand_vacuum_rel(vrel, vac_context, params.options);
old_context = MemoryContextSwitchTo(vac_context);
newrels = list_concat(newrels, sublist);
MemoryContextSwitchTo(old_context);
@@ -562,7 +560,7 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
relations = newrels;
}
else
- relations = get_all_vacuum_rels(vac_context, params->options);
+ relations = get_all_vacuum_rels(vac_context, params.options);
/*
* Decide whether we need to start/commit our own transactions.
@@ -578,11 +576,11 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
* transaction block, and also in an autovacuum worker, use own
* transactions so we can release locks sooner.
*/
- if (params->options & VACOPT_VACUUM)
+ if (params.options & VACOPT_VACUUM)
use_own_xacts = true;
else
{
- Assert(params->options & VACOPT_ANALYZE);
+ Assert(params.options & VACOPT_ANALYZE);
if (AmAutoVacuumWorkerProcess())
use_own_xacts = true;
else if (in_outer_xact)
@@ -633,21 +631,13 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
{
VacuumRelation *vrel = lfirst_node(VacuumRelation, cur);
- if (params->options & VACOPT_VACUUM)
+ if (params.options & VACOPT_VACUUM)
{
- VacuumParams params_copy;
-
- /*
- * vacuum_rel() scribbles on the parameters, so give it a copy
- * to avoid affecting other relations.
- */
- memcpy(¶ms_copy, params, sizeof(VacuumParams));
-
- if (!vacuum_rel(vrel->oid, vrel->relation, ¶ms_copy, bstrategy))
+ if (!vacuum_rel(vrel->oid, vrel->relation, params, bstrategy))
continue;
}
- if (params->options & VACOPT_ANALYZE)
+ if (params.options & VACOPT_ANALYZE)
{
/*
* If using separate xacts, start one for analyze. Otherwise,
@@ -711,8 +701,8 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy,
StartTransactionCommand();
}
- if ((params->options & VACOPT_VACUUM) &&
- !(params->options & VACOPT_SKIP_DATABASE_STATS))
+ if ((params.options & VACOPT_VACUUM) &&
+ !(params.options & VACOPT_SKIP_DATABASE_STATS))
{
/*
* Update pg_database.datfrozenxid, and truncate pg_xact if possible.
@@ -1110,7 +1100,7 @@ get_all_vacuum_rels(MemoryContext vac_context, int options)
* minimum).
*/
bool
-vacuum_get_cutoffs(Relation rel, const VacuumParams *params,
+vacuum_get_cutoffs(Relation rel, const VacuumParams params,
struct VacuumCutoffs *cutoffs)
{
int freeze_min_age,
@@ -1126,10 +1116,10 @@ vacuum_get_cutoffs(Relation rel, const VacuumParams *params,
aggressiveMXIDCutoff;
/* Use mutable copies of freeze age parameters */
- freeze_min_age = params->freeze_min_age;
- multixact_freeze_min_age = params->multixact_freeze_min_age;
- freeze_table_age = params->freeze_table_age;
- multixact_freeze_table_age = params->multixact_freeze_table_age;
+ freeze_min_age = params.freeze_min_age;
+ multixact_freeze_min_age = params.multixact_freeze_min_age;
+ freeze_table_age = params.freeze_table_age;
+ multixact_freeze_table_age = params.multixact_freeze_table_age;
/* Set pg_class fields in cutoffs */
cutoffs->relfrozenxid = rel->rd_rel->relfrozenxid;
@@ -2006,7 +1996,7 @@ vac_truncate_clog(TransactionId frozenXID,
* At entry and exit, we are not inside a transaction.
*/
static bool
-vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
+vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
BufferAccessStrategy bstrategy)
{
LOCKMODE lmode;
@@ -2019,18 +2009,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
int save_nestlevel;
VacuumParams toast_vacuum_params;
- Assert(params != NULL);
-
/*
* This function scribbles on the parameters, so make a copy early to
* avoid affecting the TOAST table (if we do end up recursing to it).
*/
- memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
+ memcpy(&toast_vacuum_params, ¶ms, sizeof(VacuumParams));
/* Begin a transaction for vacuuming this relation */
StartTransactionCommand();
- if (!(params->options & VACOPT_FULL))
+ if (!(params.options & VACOPT_FULL))
{
/*
* In lazy vacuum, we can set the PROC_IN_VACUUM flag, which lets
@@ -2056,7 +2044,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
*/
LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
MyProc->statusFlags |= PROC_IN_VACUUM;
- if (params->is_wraparound)
+ if (params.is_wraparound)
MyProc->statusFlags |= PROC_VACUUM_FOR_WRAPAROUND;
ProcGlobal->statusFlags[MyProc->pgxactoff] = MyProc->statusFlags;
LWLockRelease(ProcArrayLock);
@@ -2080,12 +2068,12 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
* vacuum, but just ShareUpdateExclusiveLock for concurrent vacuum. Either
* way, we can be sure that no other backend is vacuuming the same table.
*/
- lmode = (params->options & VACOPT_FULL) ?
+ lmode = (params.options & VACOPT_FULL) ?
AccessExclusiveLock : ShareUpdateExclusiveLock;
/* open the relation and get the appropriate lock on it */
- rel = vacuum_open_relation(relid, relation, params->options,
- params->log_min_duration >= 0, lmode);
+ rel = vacuum_open_relation(relid, relation, params.options,
+ params.log_min_duration >= 0, lmode);
/* leave if relation could not be opened or locked */
if (!rel)
@@ -2100,8 +2088,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
* This is only safe to do because we hold a session lock on the main
* relation that prevents concurrent deletion.
*/
- if (OidIsValid(params->toast_parent))
- priv_relid = params->toast_parent;
+ if (OidIsValid(params.toast_parent))
+ priv_relid = params.toast_parent;
else
priv_relid = RelationGetRelid(rel);
@@ -2114,7 +2102,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
*/
if (!vacuum_is_permitted_for_relation(priv_relid,
rel->rd_rel,
- params->options & ~VACOPT_ANALYZE))
+ params.options & ~VACOPT_ANALYZE))
{
relation_close(rel, lmode);
PopActiveSnapshot();
@@ -2185,7 +2173,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
* Set index_cleanup option based on index_cleanup reloption if it wasn't
* specified in VACUUM command, or when running in an autovacuum worker
*/
- if (params->index_cleanup == VACOPTVALUE_UNSPECIFIED)
+ if (params.index_cleanup == VACOPTVALUE_UNSPECIFIED)
{
StdRdOptIndexCleanup vacuum_index_cleanup;
@@ -2196,23 +2184,23 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
((StdRdOptions *) rel->rd_options)->vacuum_index_cleanup;
if (vacuum_index_cleanup == STDRD_OPTION_VACUUM_INDEX_CLEANUP_AUTO)
- params->index_cleanup = VACOPTVALUE_AUTO;
+ params.index_cleanup = VACOPTVALUE_AUTO;
else if (vacuum_index_cleanup == STDRD_OPTION_VACUUM_INDEX_CLEANUP_ON)
- params->index_cleanup = VACOPTVALUE_ENABLED;
+ params.index_cleanup = VACOPTVALUE_ENABLED;
else
{
Assert(vacuum_index_cleanup ==
STDRD_OPTION_VACUUM_INDEX_CLEANUP_OFF);
- params->index_cleanup = VACOPTVALUE_DISABLED;
+ params.index_cleanup = VACOPTVALUE_DISABLED;
}
}
#ifdef USE_INJECTION_POINTS
- if (params->index_cleanup == VACOPTVALUE_AUTO)
+ if (params.index_cleanup == VACOPTVALUE_AUTO)
INJECTION_POINT("vacuum-index-cleanup-auto", NULL);
- else if (params->index_cleanup == VACOPTVALUE_DISABLED)
+ else if (params.index_cleanup == VACOPTVALUE_DISABLED)
INJECTION_POINT("vacuum-index-cleanup-disabled", NULL);
- else if (params->index_cleanup == VACOPTVALUE_ENABLED)
+ else if (params.index_cleanup == VACOPTVALUE_ENABLED)
INJECTION_POINT("vacuum-index-cleanup-enabled", NULL);
#endif
@@ -2222,36 +2210,36 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
*/
if (rel->rd_options != NULL &&
((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate >= 0)
- params->max_eager_freeze_failure_rate =
+ params.max_eager_freeze_failure_rate =
((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate;
/*
* Set truncate option based on truncate reloption or GUC if it wasn't
* specified in VACUUM command, or when running in an autovacuum worker
*/
- if (params->truncate == VACOPTVALUE_UNSPECIFIED)
+ if (params.truncate == VACOPTVALUE_UNSPECIFIED)
{
StdRdOptions *opts = (StdRdOptions *) rel->rd_options;
if (opts && opts->vacuum_truncate_set)
{
if (opts->vacuum_truncate)
- params->truncate = VACOPTVALUE_ENABLED;
+ params.truncate = VACOPTVALUE_ENABLED;
else
- params->truncate = VACOPTVALUE_DISABLED;
+ params.truncate = VACOPTVALUE_DISABLED;
}
else if (vacuum_truncate)
- params->truncate = VACOPTVALUE_ENABLED;
+ params.truncate = VACOPTVALUE_ENABLED;
else
- params->truncate = VACOPTVALUE_DISABLED;
+ params.truncate = VACOPTVALUE_DISABLED;
}
#ifdef USE_INJECTION_POINTS
- if (params->truncate == VACOPTVALUE_AUTO)
+ if (params.truncate == VACOPTVALUE_AUTO)
INJECTION_POINT("vacuum-truncate-auto", NULL);
- else if (params->truncate == VACOPTVALUE_DISABLED)
+ else if (params.truncate == VACOPTVALUE_DISABLED)
INJECTION_POINT("vacuum-truncate-disabled", NULL);
- else if (params->truncate == VACOPTVALUE_ENABLED)
+ else if (params.truncate == VACOPTVALUE_ENABLED)
INJECTION_POINT("vacuum-truncate-enabled", NULL);
#endif
@@ -2261,9 +2249,9 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
* automatically rebuilt by cluster_rel so we shouldn't recurse to it,
* unless PROCESS_MAIN is disabled.
*/
- if ((params->options & VACOPT_PROCESS_TOAST) != 0 &&
- ((params->options & VACOPT_FULL) == 0 ||
- (params->options & VACOPT_PROCESS_MAIN) == 0))
+ if ((params.options & VACOPT_PROCESS_TOAST) != 0 &&
+ ((params.options & VACOPT_FULL) == 0 ||
+ (params.options & VACOPT_PROCESS_MAIN) == 0))
toast_relid = rel->rd_rel->reltoastrelid;
else
toast_relid = InvalidOid;
@@ -2286,16 +2274,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
* table is required (e.g., PROCESS_TOAST is set), we force PROCESS_MAIN
* to be set when we recurse to the TOAST table.
*/
- if (params->options & VACOPT_PROCESS_MAIN)
+ if (params.options & VACOPT_PROCESS_MAIN)
{
/*
* Do the actual work --- either FULL or "lazy" vacuum
*/
- if (params->options & VACOPT_FULL)
+ if (params.options & VACOPT_FULL)
{
ClusterParams cluster_params = {0};
- if ((params->options & VACOPT_VERBOSE) != 0)
+ if ((params.options & VACOPT_VERBOSE) != 0)
cluster_params.options |= CLUOPT_VERBOSE;
/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
@@ -2342,7 +2330,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
toast_vacuum_params.options |= VACOPT_PROCESS_MAIN;
toast_vacuum_params.toast_parent = relid;
- vacuum_rel(toast_relid, NULL, &toast_vacuum_params, bstrategy);
+ vacuum_rel(toast_relid, NULL, toast_vacuum_params, bstrategy);
}
/*
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 451fb90a610a..9474095f271a 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -3190,7 +3190,7 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
rel_list = list_make1(rel);
MemoryContextSwitchTo(old_context);
- vacuum(rel_list, &tab->at_params, bstrategy, vac_context, true);
+ vacuum(rel_list, tab->at_params, bstrategy, vac_context, true);
MemoryContextDelete(vac_context);
}
--
2.50.0
diff --git a/src/test/regress/expected/vacuum.out
b/src/test/regress/expected/vacuum.out
index 0abcc99989e0..a4d9c5510cfc 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -686,3 +686,61 @@ RESET ROLE;
DROP TABLE vacowned;
DROP TABLE vacowned_parted;
DROP ROLE regress_vacuum;
+-- TRUNCATE option with VACUUM of more than 1 relation.
+CREATE TABLE vac_truncate_on_toast_off(i int, j text) WITH
+ (autovacuum_enabled=false, vacuum_truncate=true,
toast.vacuum_truncate=false);
+CREATE TABLE vac_truncate_off_toast_on(i int, j text) WITH
+ (autovacuum_enabled=false, vacuum_truncate=false,
toast.vacuum_truncate=true);
+-- EXTERNAL to force data on TOAST table, uncompressed.
+ALTER TABLE vac_truncate_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL;
+ALTER TABLE vac_truncate_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL;
+INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1, 1000), NULL;
+INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1000, 1010),
repeat('1234567890', 1000);
+INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1, 1000), NULL;
+INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1000, 1010),
repeat('1234567890', 1000);
+SELECT pg_relation_size('vac_truncate_on_toast_off') > 0 AS has_data;
+ has_data
+----------
+ t
+(1 row)
+
+SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class
+ WHERE relname = 'vac_truncate_on_toast_off';
+ has_data
+----------
+ t
+(1 row)
+
+SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data;
+ has_data
+----------
+ t
+(1 row)
+
+SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class
+ WHERE relname = 'vac_truncate_off_toast_on';
+ has_data
+----------
+ t
+(1 row)
+
+DELETE FROM vac_truncate_on_toast_off;
+DELETE FROM vac_truncate_off_toast_on;
+-- TRUNCATE options are retrieved from their respective relations.
+-- Do an aggressive VACUUM to prevent page-skipping.
+VACUUM (FREEZE, ANALYZE) vac_truncate_on_toast_off, vac_truncate_off_toast_on;
+SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class
+ WHERE relname = 'vac_truncate_on_toast_off';
+ has_data
+----------
+ t
+(1 row)
+
+SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data;
+ has_data
+----------
+ t
+(1 row)
+
+DROP TABLE vac_truncate_on_toast_off;
+DROP TABLE vac_truncate_off_toast_on;
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index a72bdb5b619d..865582f93d17 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -495,3 +495,32 @@ RESET ROLE;
DROP TABLE vacowned;
DROP TABLE vacowned_parted;
DROP ROLE regress_vacuum;
+
+-- TRUNCATE option with VACUUM of more than 1 relation.
+CREATE TABLE vac_truncate_on_toast_off(i int, j text) WITH
+ (autovacuum_enabled=false, vacuum_truncate=true,
toast.vacuum_truncate=false);
+CREATE TABLE vac_truncate_off_toast_on(i int, j text) WITH
+ (autovacuum_enabled=false, vacuum_truncate=false,
toast.vacuum_truncate=true);
+-- EXTERNAL to force data on TOAST table, uncompressed.
+ALTER TABLE vac_truncate_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL;
+ALTER TABLE vac_truncate_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL;
+INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1, 1000), NULL;
+INSERT INTO vac_truncate_on_toast_off SELECT generate_series(1000, 1010),
repeat('1234567890', 1000);
+INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1, 1000), NULL;
+INSERT INTO vac_truncate_off_toast_on SELECT generate_series(1000, 1010),
repeat('1234567890', 1000);
+SELECT pg_relation_size('vac_truncate_on_toast_off') > 0 AS has_data;
+SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class
+ WHERE relname = 'vac_truncate_on_toast_off';
+SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data;
+SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class
+ WHERE relname = 'vac_truncate_off_toast_on';
+DELETE FROM vac_truncate_on_toast_off;
+DELETE FROM vac_truncate_off_toast_on;
+-- TRUNCATE options are retrieved from their respective relations.
+-- Do an aggressive VACUUM to prevent page-skipping.
+VACUUM (FREEZE, ANALYZE) vac_truncate_on_toast_off, vac_truncate_off_toast_on;
+SELECT pg_relation_size(reltoastrelid) > 0 AS has_data FROM pg_class
+ WHERE relname = 'vac_truncate_on_toast_off';
+SELECT pg_relation_size('vac_truncate_off_toast_on') > 0 AS has_data;
+DROP TABLE vac_truncate_on_toast_off;
+DROP TABLE vac_truncate_off_toast_on;
diff --git a/contrib/pgstattuple/expected/pgstattuple.out
b/contrib/pgstattuple/expected/pgstattuple.out
index 9176dc98b6a9..f0bdf7717e30 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -303,3 +303,50 @@ drop view test_view;
drop foreign table test_foreign_table;
drop server dummy_server;
drop foreign data wrapper dummy;
+-- INDEX_CLEANUP option with VACUUM of more than 1 relation.
+CREATE TABLE vac_index_cleanup_on_toast_off(i int primary key, j text) WITH
+ (autovacuum_enabled=false, vacuum_index_cleanup=true,
toast.vacuum_index_cleanup=false);
+CREATE TABLE vac_index_cleanup_off_toast_on(i int primary key, j text)
+ WITH (autovacuum_enabled=false, vacuum_index_cleanup=false,
toast.vacuum_index_cleanup=true);
+-- EXTERNAL to force data on TOAST table, uncompressed.
+ALTER TABLE vac_index_cleanup_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL;
+ALTER TABLE vac_index_cleanup_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL;
+INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1, 1000),
NULL;
+INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1001, 1010),
repeat('1234567890', 10000);
+INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1, 1000),
NULL;
+INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1001, 1010),
repeat('1234567890', 10000);
+DELETE FROM vac_index_cleanup_on_toast_off;
+DELETE FROM vac_index_cleanup_off_toast_on;
+-- Do an aggressive VACUUM to prevent page-skipping
+VACUUM (FREEZE, ANALYZE) vac_index_cleanup_on_toast_off,
vac_index_cleanup_off_toast_on;
+-- Check cleanup state of main table indexes
+SELECT deleted_pages > 0 AS has_del_pages FROM
pgstatindex('vac_index_cleanup_off_toast_on_pkey');
+ has_del_pages
+---------------
+ f
+(1 row)
+
+SELECT deleted_pages > 0 AS has_del_pages FROM
pgstatindex('vac_index_cleanup_on_toast_off_pkey');
+ has_del_pages
+---------------
+ t
+(1 row)
+
+-- Check cleanup state of TOAST indexes
+WITH index_data AS (
+ SELECT indexrelid::regclass AS indname, c.relname AS tabname FROM pg_class
AS c, pg_index AS i
+ WHERE c.reltoastrelid = i.indrelid AND
+ c.relname IN ('vac_index_cleanup_off_toast_on',
+ 'vac_index_cleanup_on_toast_off')
+)
+SELECT tabname, deleted_pages > 0 AS has_del_pages
+ FROM index_data, pgstatindex(index_data.indname)
+ ORDER BY tabname COLLATE "C";
+ tabname | has_del_pages
+--------------------------------+---------------
+ vac_index_cleanup_off_toast_on | t
+ vac_index_cleanup_on_toast_off | f
+(2 rows)
+
+DROP TABLE vac_index_cleanup_on_toast_off;
+DROP TABLE vac_index_cleanup_off_toast_on;
diff --git a/contrib/pgstattuple/sql/pgstattuple.sql
b/contrib/pgstattuple/sql/pgstattuple.sql
index 7e72c567a064..9a0d284f9b14 100644
--- a/contrib/pgstattuple/sql/pgstattuple.sql
+++ b/contrib/pgstattuple/sql/pgstattuple.sql
@@ -136,3 +136,35 @@ drop view test_view;
drop foreign table test_foreign_table;
drop server dummy_server;
drop foreign data wrapper dummy;
+
+-- INDEX_CLEANUP option with VACUUM of more than 1 relation.
+CREATE TABLE vac_index_cleanup_on_toast_off(i int primary key, j text) WITH
+ (autovacuum_enabled=false, vacuum_index_cleanup=true,
toast.vacuum_index_cleanup=false);
+CREATE TABLE vac_index_cleanup_off_toast_on(i int primary key, j text)
+ WITH (autovacuum_enabled=false, vacuum_index_cleanup=false,
toast.vacuum_index_cleanup=true);
+-- EXTERNAL to force data on TOAST table, uncompressed.
+ALTER TABLE vac_index_cleanup_on_toast_off ALTER COLUMN j SET STORAGE EXTERNAL;
+ALTER TABLE vac_index_cleanup_off_toast_on ALTER COLUMN j SET STORAGE EXTERNAL;
+INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1, 1000),
NULL;
+INSERT INTO vac_index_cleanup_on_toast_off SELECT generate_series(1001, 1010),
repeat('1234567890', 10000);
+INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1, 1000),
NULL;
+INSERT INTO vac_index_cleanup_off_toast_on SELECT generate_series(1001, 1010),
repeat('1234567890', 10000);
+DELETE FROM vac_index_cleanup_on_toast_off;
+DELETE FROM vac_index_cleanup_off_toast_on;
+-- Do an aggressive VACUUM to prevent page-skipping
+VACUUM (FREEZE, ANALYZE) vac_index_cleanup_on_toast_off,
vac_index_cleanup_off_toast_on;
+-- Check cleanup state of main table indexes
+SELECT deleted_pages > 0 AS has_del_pages FROM
pgstatindex('vac_index_cleanup_off_toast_on_pkey');
+SELECT deleted_pages > 0 AS has_del_pages FROM
pgstatindex('vac_index_cleanup_on_toast_off_pkey');
+-- Check cleanup state of TOAST indexes
+WITH index_data AS (
+ SELECT indexrelid::regclass AS indname, c.relname AS tabname FROM pg_class
AS c, pg_index AS i
+ WHERE c.reltoastrelid = i.indrelid AND
+ c.relname IN ('vac_index_cleanup_off_toast_on',
+ 'vac_index_cleanup_on_toast_off')
+)
+SELECT tabname, deleted_pages > 0 AS has_del_pages
+ FROM index_data, pgstatindex(index_data.indname)
+ ORDER BY tabname COLLATE "C";
+DROP TABLE vac_index_cleanup_on_toast_off;
+DROP TABLE vac_index_cleanup_off_toast_on;
signature.asc
Description: PGP signature
