Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 20 Jun 2025 10:34:19 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: shihao zhong <zhong950419@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Fixes inconsistent behavior in vacuum when it processes multiple
 relations
Message-ID: <aFS6m1tNH2sGMQ61@paquier.xyz>
References: 
 <CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com>
 <aFLp9sMEHFHULOFx@nathan>
 <CAGRkXqTp9k_7X1OqqQSnVfoyGxAM3SQ083CcyaezBg9qQ8i+qg@mail.gmail.com>
 <aFRzYhOTZcRgKPLu@nathan>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="qzwoxyrzjIJdot+n"
Content-Disposition: inline
In-Reply-To: <aFRzYhOTZcRgKPLu@nathan>
Archived-At: 
 <https://www.postgresql.org/message-id/aFS6m1tNH2sGMQ61%40paquier.xyz>
Precedence: bulk


--qzwoxyrzjIJdot+n
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jun 19, 2025 at 03:30:26PM -0500, Nathan Bossart wrote:
> 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 simil=
ar
> problem when recursing to the TOAST table, which can be fixed by copying
> the params at the top of vacuum_rel().
>=20
> While testing out the attached patch, I noticed a couple of other
> interesting problems in this area [0].
>=20
> [0] https://postgr.es/m/aFRxC1W_kZU9OjJ9%40nathan

Hmm.  I like the simplicity of option 2) for the purpose the back
branches and the post-feature-freeze v18.

However, Option 1) would be my go-to option for HEAD (as of v19
opening for business), but I think that we should harden the code more
than suggested and treat all VacuumParams as purely input arguments
rather keeping some pointers to it depending on the code path we are
dealing with, so as no callers of these inner routines is surprised by
changes that may happen internally.  Hence, reading the code of v2,
I'd suggest to apply the same rule to vacuum_get_cutoffs(),
do_analyze_rel() and heap_vacuum_rel().  Except if I am missing
something, it looks like all these calls should be OK with this new
policy.  This implies also changing relation_vacuum() in tableam.h,
which can be a HEAD-only change anyway.
--
Michael

--qzwoxyrzjIJdot+n
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmhUupsACgkQnvQgOdby
QH220A/+PezhKyyEeeahk6/btQG03LYhJ7qJM0GyuFCkm5pmKWWA/0jh8WocjZAX
YZ0Ja6B4+CJb0NN0ORbWM/WcWParFz0xILc4ZIiSCbMtznWgMvFYl7poNVGzEQIq
6gW96kcsXZgdN7poIzn2BG9PL6BXaSSWG5nQniYOht1ULBWR91panccFZjKU236R
rq4w9UbesQFNI3rqgqtvqXK8wkF1cP1rxxdWHwB76XEUzx/PoOnu4RShISa7eUVB
CO9ZFDo9ICiPzo6L1C4ex6W4p3MJf9pMmlJ1neTlpaF4GsqMNDITS5exmnJ4RnR0
eqXIoou0UU50GpcI+jwLoSj0/pbHHwQg6XTmJ8BAY/9z/m4tQ98f5Xh45MBsvTkF
1WaVbgiNN8xtB0gKVpyKBNMd5hH+irgKLRrHky79v5+nNE0s4lxHVmd/47Y0QvOY
3yjwNZVV9UZ6wrsgaNsnS0zVTvYrhl2ZfMbMuy4/9eUauzoBMRUqFcJwYwPV0hgQ
0Y5BbafaqIw3a1zZW/P4PL6sSzV2iCNb+MlR0qbBNuO9eOv+05LIgWJk8781ZkNs
jKFS0XH8kUUD5nO+7dAwklOSzMkx4FYzXRSjGV+3Y5SyncR74Cq2IypuYG25oBE4
8jVn64/BM+5tNzjSzAI7ImK7cSX5xIFhKF+1jReCvZzWmSCjyhg=
=FiYH
-----END PGP SIGNATURE-----

--qzwoxyrzjIJdot+n--