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

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 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

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