Re: problems with toast.* reloptions

On Thu, Jun 19, 2025 at 03:20:27PM -0500, Nathan Bossart wrote:
> While investigating problems caused by vacuum_rel() scribbling on its
> VacuumParams argument [0], I noticed some other interesting bugs with the
> toast.* reloption code. Note that the documentation for the reloptions has
> the following line:
>
> If a table parameter value is set and the equivalent toast. parameter
> is not, the TOAST table will use the table's parameter value.
>
> The problems I found are as follows:
>
> * vacuum_rel() does not look up the main relation's reloptions when
> processing a TOAST table, which is a problem for manual VACUUMs. The
> aforementioned bug [0] causes you to sometimes get the expected behavior
> (because the parameters are overridden before recursing to TOAST), but
> fixing that bug makes that accidental behavior go away.

Are you referring to the case of a VACUUM pg_toast.pg_toast_NNN? I'm
not sure that we really need to care about looking up at the parent
relation in this case. It sounds to me that the intention of this
paragraph is for the case where the TOAST table is treated as a
secondary table, not when the TOAST table is directly vacuumed.
Perhaps the wording of the docs should be improved that this does not
happen if vacuuming directly a TOAST table.

> * For autovacuum, the main table's reloptions are only used if the TOAST
> table has no reloptions set. So, if your relation has
> autovacuum_vacuum_threshold and toast.vacuum_index_cleanup set, the main
> relation's autovacuum_vacuum_threshold setting won't be used for the
> TOAST table.

[.. /me double-checks the code .. ]

So we combine the options in do_autovacuum() with the two-pass logic
to gather the relation OIDs, then apply relation_needs_vacanalyze().
That looks like an old issue, that cannot be solved as long as we rely
on the relopts to be an all-or-nothing thing when assigning the
individual values for the TOAST relation. Oops.

> * Even when the preceding point doesn't apply, autovacuum doesn't use the
> main relation's setting for some parameters (e.g., vacuum_truncate).
> Instead, it leaves them uninitialized and expects vacuum_rel() to fill
> them in. This is a problem because, as mentioned earlier, vacuum_rel()
> doesn't consult the main relation's reloptions either.

Relying on vacuum_rel() sounds like a bad idea if we can avoid that,
still perhaps that's OK as long as we don't use a pointer to the
VacuumParams and keep the updates to the value of vacuum_rel() local
inside the routine.

> I think we need to do something like the following to fix this:
>
> * Teach autovacuum to combine the TOAST reloptions with the main relation's
> when processing TOAST tables (with the toast.* ones winning if both are
> set).
>
> * Teach autovacuum to resolve reloptions for parameters like
> vacuum_truncate instead of relying on vacuum_rel() to fill it in.

These two points make sense here, yes.

> * Have vacuum_rel() send the main relation's reloptions when recursing to
> the TOAST table so that we can combine them there, too.

For the case of a manual VACUUM on the main table, where the TOAST
table is treated as a secondary citizen, that makes sense as well,
yes.

> This doesn't fix VACUUM against a TOAST table directly (e.g., VACUUM
> pg_toast.pg_toast_5432), but that might not be too important because
> (PROCESS_TOAST TRUE) is the main supported way to vacuum a TOAST table. If
> we did want to fix that, though, I think we'd have to teach vacuum_rel() or
> the relcache to look up the reloptions for the main relation.

This one does not sound that important to me for the case of manual
VACUUM case directly done on a TOAST table. If you do that, the code
kind of assumes that a TOAST table is actually a "main" relation that
has no TOAST table. That should keep the code simpler, because we
would not need to look at what the parent relation holds when deciding
which options to use in ExecVacuum(). The autovacuum case is
different, as TOAST relations are worked on as their own items rather
than being secondary relations of the main tables.
--
Michael