postgres.git / summary / log / commit / refs

commit    65c3223f93a0e5dc4ae1f3ec5fd48455b2da8d86
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Wed Jun 25 01:03:56 2025 +0000
First released in: 18.0, 13.22, 14.19, 15.14, 16.10, 17.6

    Avoid scribbling of VACUUM options
    
    This fixes two issues with the handling of VacuumParams in vacuum_rel().
    This code path has the idea to change the passed-in pointer of
    VacuumParams for the "truncate" and "index_cleanup" options for the
    relation worked on, impacting the two following scenarios where
    incorrect options may be used because a VacuumParams pointer is shared
    across multiple relations:
    - Multiple relations in a single VACUUM command.
    - TOAST relations vacuumed with their main relation.
    
    The problem is avoided by providing to the two callers of vacuum_rel()
    copies of VacuumParams, before the pointer is updated for the "truncate"
    and "index_cleanup" options.
    
    The refactoring of the VACUUM option and parameters done in 0d831389749a
    did not introduce an issue, but it has encouraged the problem we are
    dealing with in this commit, with b84dbc8eb80b for "truncate" and
    a96c41feec6b for "index_cleanup" that have been added a couple of years
    after the initial refactoring.  HEAD will be improved with a different
    patch that hardens the uses of VacuumParams across the tree.  This
    cannot be backpatched as it introduces an ABI breakage.
    
    The backend portion of the patch has been authored by Nathan, while I
    have implemented the tests.  The tests rely on injection points to check
    the option values, making them faster, more reliable than the tests
    originally proposed by Shihao, and they also provide more coverage.
    This part can only be backpatched down to v17.
    
    Reported-by: Shihao Zhong <zhong950419@gmail.com>
    Author: Nathan Bossart <nathandbossart@gmail.com>
    Co-authored-by: Michael Paquier <michael@paquier.xyz>
    Discussion: https://postgr.es/m/CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com
    Backpatch-through: 13


 src/backend/commands/vacuum.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 1fa71d11bcf..53aeb9ffcc4 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -447,7 +447,14 @@ vacuum(List *relations, VacuumParams *params,
 
 			if (params->options & VACOPT_VACUUM)
 			{
-				if (!vacuum_rel(vrel->oid, vrel->relation, params))
+				VacuumParams params_copy;
+
+				/*
+				 * vacuum_rel() scribbles on the parameters, so give it a copy
+				 * to avoid affecting other relations.
+				 */
+				memcpy(&params_copy, params, sizeof(VacuumParams));
+				if (!vacuum_rel(vrel->oid, vrel->relation, &params_copy))
 					continue;
 			}
 
@@ -1738,9 +1745,16 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	Oid			save_userid;
 	int			save_sec_context;
 	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));
+
 	/* Begin a transaction for vacuuming this relation */
 	StartTransactionCommand();
 
@@ -1967,7 +1981,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	 * totally unimportant for toast relations.
 	 */
 	if (toast_relid != InvalidOid)
-		vacuum_rel(toast_relid, NULL, params);
+		vacuum_rel(toast_relid, NULL, &toast_vacuum_params);
 
 	/*
 	 * Now release the session-level lock on the master table.


[parent: 87819f766f37]