Re: problems with toast.* reloptions - Mailing list pgsql-hackers

On Fri, Aug 7, 2026, at 12:50 PM, Nathan Bossart wrote:
> On Thu, Aug 06, 2026 at 04:38:28PM -0500, Nathan Bossart wrote:
>> Thanks for looking.  Here is a new patch set with some small fixes.
>
> Sorry for the noise.  I found some other small bugs and found another small
> cleanup opportunity.

Hey Nathan,

First, thanks for taking on this subtle and confusing aspect of heap/TOAST/reloptions interactions.  I agree with your
diagnosisand your approach and I think this would be a solid step in the right direction.  I applied your v7 patch set
andran the test world, works as advertised.
 

Am I misunderstanding this? It seems to me that making autovacuum_enabled a ternary and then merging it means a heap
tablewith autovacuum_enabled=false and some toast.* option set now stops autovacuuming the TOAST table.
 
  
In v7-0002 the option moves from bool to ternary:
  
    --- a/src/include/utils/rel.h
    +++ b/src/include/utils/rel.h
    @@ typedef struct AutoVacOpts
    - bool    enabled;
    + pg_ternary  enabled;
  
    --- a/src/backend/postmaster/autovacuum.c
    +++ b/src/backend/postmaster/autovacuum.c
    @@ relation_needs_vacanalyze
    - av_enabled = (avopts ? avopts->enabled : true);
    + av_enabled = (avopts ? avopts->enabled != PG_TERNARY_FALSE : true);
  
and in v7-0005 the merge fills an unset TOAST value from the main table:
  
    + /* ternary fields */
    + static const int ternary_offsets[] = {
    +   offsetof(AutoVacOpts, enabled),
    + };
    ...
    + for (int i = 0; i < lengthof(ternary_offsets); i++)
    + {
    +   pg_ternary *toast_opt;
    +   pg_ternary *main_opt;
    +
    +   toast_opt = (pg_ternary *) ((char *) toast_avopts + ternary_offsets[i]);
    +   main_opt = (pg_ternary *) ((char *) main_avopts + ternary_offsets[i]);
    +
    +   if (*toast_opt == PG_TERNARY_UNSET)
    +     *toast_opt = *main_opt;
    + }
  
So main enabled=PG_TERNARY_FALSE + toast unset -> toast enabled becomes PG_TERNARY_FALSE -> av_enabled is false.
Today'sall-or-nothing bug leaves that TOAST table getting vacuumed. I agree the new behavior matches the documented
contract,but it is a behavior change for the person who disabled autovac on a table they vacuum by hand and never
thoughtabout the TOAST side. Wraparound is still forced, but ordinary dead-tuple bloat on the TOAST relation is now on
them.So, maybe a line in the commit message and in the CREATE TABLE docs to make that more explicit would help people
avoidmaking that mistake in practice?
 
  
In merge_autovac_opts() the four offset arrays keyed by "which sentinel means unset", is that duplicating knowledge
thatalready lives in the relopt tables in reloptions.c?
 
  
    + /* integer fields whose unset sentinel is -1 */
    + static const int int_offsets_1[] = {
    +   offsetof(AutoVacOpts, vacuum_threshold),
    +   offsetof(AutoVacOpts, vacuum_cost_limit),
    +   offsetof(AutoVacOpts, freeze_min_age),
    +   offsetof(AutoVacOpts, freeze_max_age),
    +   offsetof(AutoVacOpts, freeze_table_age),
    +   offsetof(AutoVacOpts, multixact_freeze_min_age),
    +   offsetof(AutoVacOpts, multixact_freeze_max_age),
    +   offsetof(AutoVacOpts, multixact_freeze_table_age),
    + };
    +
    + /* integer fields whose unset sentinel is -2 */
    + static const int int_offsets_2[] = {
    +   offsetof(AutoVacOpts, vacuum_max_threshold),
    +   offsetof(AutoVacOpts, vacuum_ins_threshold),
    +   offsetof(AutoVacOpts, log_vacuum_min_duration),
    + };
  
Those -1/-2 sentinels are the very defaults declared over in reloptions.c this same patch even moves one of them from
-1to -2 there:
 
  
    --- a/src/backend/access/common/reloptions.c
    +++ b/src/backend/access/common/reloptions.c
    @@ static relopt_int intRelOpts[] =
          RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
          ShareUpdateExclusiveLock
        },
    -   -1, -1, INT_MAX
    +   -2, -1, INT_MAX
  
so the "which sentinel means unset for field X" fact now lives in two places, kept in agreement only by the NB comment
addedin rel.h:
 
  
    + * NB: When adding a new member, be sure to update merge_autovac_opts() and/or
    + * table_recheck_autovac() as necessary!
  
Add an AutoVacOpts field, or change a field's default sentinel, and forget to update the matching array here, and the
mergesilently keeps the TOAST table's default instead of inheriting, nothing fails to compile and no test goes red.
Canthis be driven off the relopt metadata (relopt_parse_elt already knows each option's type and default) instead of
thehand-maintained offset arrays?
 
  
On testing: the coverage doesn't touch the risky code. There's one injection-point case, and it's manual VACUUM only,
index_cleanup/truncateonly:
 
  
    +-- TOAST table inherits main table's resolved values
    +CREATE TABLE vac_tab_toast_inherit(i int, j text) WITH
    +  (autovacuum_enabled=false,
    +   vacuum_index_cleanup=false,
    +   vacuum_truncate=false, toast.vacuum_truncate=true);
    +VACUUM vac_tab_toast_inherit;
    +DROP TABLE vac_tab_toast_inherit;
  
Nothing exercises the autovacuum decision path, autovacuum_enabled inheritance, or any of the numeric AutoVacOpts that
merge_autovac_opts()actually resolves which is precisely the code I'm worried about above. FWIW the
pg_stat_get_autovacuum_scores()SRF that 0005 extends looks like it could drive a deterministic test of the autovac path
(computethe decision without spawning a worker), which sidesteps the flakiness worry raised upthread.
 

In summary, solid work and I hope it lands.  Just a few small issues to clean up.

best.

-greg


> -- 
> nathan
>
> Attachments:
> * v7-0001-Remove-extract_autovac_opts.patch
> * v7-0002-Make-autovacuum_enabled-a-ternary-reloption.patch
> * v7-0003-Add-an-unset-value-for-vacuum_index_cleanup.patch
> * v7-0004-Simplify-autovacuum-s-TOAST-to-main-relation-relo.patch
> * v7-0005-Fix-VACUUM-and-autovacuum-handling-of-TOAST-stora.patch