MIME-Version: 1.0
References: 
 <CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com>
 <aFLp9sMEHFHULOFx@nathan>
 <CAGRkXqTp9k_7X1OqqQSnVfoyGxAM3SQ083CcyaezBg9qQ8i+qg@mail.gmail.com>
 <aFRzYhOTZcRgKPLu@nathan> <aFS6m1tNH2sGMQ61@paquier.xyz>
 <CAGRkXqR70a6hyRPmj66fOOMYtt1QQVEup3BSdw89cDzkVRBPiw@mail.gmail.com>
In-Reply-To: 
 <CAGRkXqR70a6hyRPmj66fOOMYtt1QQVEup3BSdw89cDzkVRBPiw@mail.gmail.com>
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Fri, 20 Jun 2025 22:45:44 +0800
Message-ID: 
 <CAEG8a3JzWkeCxajKs3MdyCCU1iNtjD9kMfrHnESi=KCPiyPTgA@mail.gmail.com>
Subject: Re: Fixes inconsistent behavior in vacuum when it processes multiple
 relations
To: shihao zhong <zhong950419@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>,
 Nathan Bossart <nathandbossart@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAEG8a3JzWkeCxajKs3MdyCCU1iNtjD9kMfrHnESi%3DKCPiyPTgA%40mail.gmail.com>
Precedence: bulk

On Fri, Jun 20, 2025 at 10:14=E2=80=AFPM shihao zhong <zhong950419@gmail.co=
m> wrote:
>
> >> However, Option 1) would be my go-to option for HEAD ...
>
> Updated my patch to apply the same rules to all VacuumParams. Also I
> am seeing clusterParams as a pass reference, not sure if we should
> also change that to prevent future issues. But that should be another
> patch.

 static inline void
-table_relation_vacuum(Relation rel, struct VacuumParams *params,
+table_relation_vacuum(Relation rel, struct 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 bc37a80dc74..9a8c63352da 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -336,7 +336,7 @@ extern PGDLLIMPORT int64 parallel_vacuum_worker_delay_n=
s;

 /* 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, 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,
+ VacuumParams params, List *va_cols, bool in_outer_xact,

It's a bit odd that we have both `VacuumParams *params` and
`struct VacuumParams *params`. Perhaps you could remove
the struct keyword in this patch to make it consistent.


--=20
Regards
Junwang Zhao