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

On Fri, Jun 20, 2025 at 02:16:46PM -0500, Nathan Bossart wrote:
> On Fri, Jun 20, 2025 at 10:34:19AM +0900, Michael Paquier wrote:
>> 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.
>
> Sounds reasonable to me.

Looking at the git history, the decision of being able to change
VacuumParams from the reloptions in vacuum_rel() for the index cleanup
dates back to a96c41feec6b (April 2019). It has been copied a couple
of days later for truncate in b84dbc8eb80b (8th of May 2019). The
introduction of VacuumParams comes from 0d831389749a back in 2015,
where more pointers to VacuumParams have been introduced for something
I've authored. Perhaps we should have documented better the fact that
this structure should never be manipulated with const markers back
then, but what's done is done. So I'm at the origin of the design
problem: even if the original refactoring of 0d831389749a did not
break things, it has encouraged the pattern we have to deal with today
because vacuum_rel() has begun this practice.

Anyway, here is an attempt at leaning all that. I am really tempted
to add a couple of const markers to force VacuumParams to be in
read-only mode, even if it means doing so for vacuum() but not touch
at vacuum_rel() where we double-check the reloptions for the truncate
and index cleanup options. That would be of course v19-only material.
Thoughts or opinions?
--
Michael