Re: Fixes inconsistent behavior in vacuum when it processes multiple relations

[Nathan Bossart]

On Wed, Jun 18, 2025 at 02:48:16PM -0400, shihao zhong wrote:
>>> That leads me to think (1) might be the better option, although I'm not too
>>> wild about the subtlety of the fix.
> 
> Thanks for your feedback. New patch is attached. I also updated the
> signature of do_analyze_rel for the same reason.

After thinking about this some more, I'm wondering if it would be better to
pursue option (2) because it's a little less invasive (which is important
because this will need to be back-patched).  In any case, we have a similar
problem when recursing to the TOAST table, which can be fixed by copying
the params at the top of vacuum_rel().

While testing out the attached patch, I noticed a couple of other
interesting problems in this area [0].

[0] https://postgr.es/m/aFRxC1W_kZU9OjJ9%40nathan

-- 
nathan

diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 33a33bf6b1c..a43f090ee17 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -634,7 +634,15 @@ vacuum(List *relations, VacuumParams *params, 
BufferAccessStrategy bstrategy,
 
                        if (params->options & VACOPT_VACUUM)
                        {
-                               if (!vacuum_rel(vrel->oid, vrel->relation, 
params, bstrategy))
+                               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, bstrategy))
                                        continue;
                        }
 
@@ -2008,9 +2016,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();
 
@@ -2299,15 +2314,12 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams 
*params,
         */
        if (toast_relid != InvalidOid)
        {
-               VacuumParams toast_vacuum_params;
-
                /*
                 * Force VACOPT_PROCESS_MAIN so vacuum_rel() processes it.  
Likewise,
                 * set toast_parent so that the privilege checks are done on 
the main
                 * relation.  NB: This is only safe to do because we hold a 
session
                 * lock on the main relation that prevents concurrent deletion.
                 */
-               memcpy(&toast_vacuum_params, params, sizeof(VacuumParams));
                toast_vacuum_params.options |= VACOPT_PROCESS_MAIN;
                toast_vacuum_params.toast_parent = relid;