Re: Support for 8-byte TOAST values, round two

From: "Greg Burd" <greg(at)burd(dot)me>
To: "Michael Paquier" <michael(at)paquier(dot)xyz>, "Bharath Rupireddy" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: "Yugo Nagata" <nagata(at)sraoss(dot)co(dot)jp>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Support for 8-byte TOAST values, round two
Date: 2026-09-15 15:26:11
Message-ID: 5c7124c4-c481-403c-b02d-cc80eaae94ef@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Tue, Sep 15, 2026, at 3:08 AM, Michael Paquier wrote:
> On Mon, Sep 14, 2026 at 12:26:54PM -0700, Bharath Rupireddy wrote:
>> v18-0001-Add-support-for-oid8-TOAST-values.patch: It looks good to me.
>> v18-0002-Add-battery-of-tests-related-oid8.patch: It looks good to me.
>> v18-0003-Add-support-for-TOAST-pointers-as-oid8.patch: It looks good to me.
>

Hello Michael,

> One thing that neither you nor Greg pointed out is a disturbance I
> have found in amcheck.

Fair hit, and thanks for catching it. I read that hunk across several
versions and never questioned the direction of the inference. Deriving the
expected chunk size from the vartag is backwards in a corruption checker,
since the vartag is part of what is under suspicion; the chunk_id type from
the cached toast_rel is the trustworthy side, with the vartag then checked
against it. I will be looking at amcheck differently from now on.

> To me, applying the value set in the reloption at the moment of the dump
> and not the chunk_id type feels like a natural thing to do. We still have
> time to decide which behavior makes more sense for this release, but
> choosing what the reloption holds at the time of the dump is a property
> I'd prefer keep.

I confirmed Bharath's case on f292187f375: create with oid8, load values with
the counter past 2^32, then

ALTER TABLE t8 RESET (toast_value_type);

leaves reloptions null while chunk_id stays oid8, pg_dump then emits no WITH
clause at all, and the restore comes back with an oid TOAST table. Data is
intact and the IDs are simply reassigned small, so on a table this size
nothing is lost.

The part that gives me pause is the interaction with the case you raised
upthread yourself: an oid8 table with more than four billion live values,
dumped and restored into an oid TOAST table, spins in
GetNewOidWithIndex(). RESET is the one step that gets a user there without
ever asking for it. The dump carries no trace of the property that was
dropped, so the first sign of trouble is a restore that does not finish, on
exactly the tables big enough to have needed oid8.

I do not think that argues against your semantics for the reloption in
general, which I agree read naturally. It argues that a reset on a relation
whose TOAST relation is already oid8 is the case worth treating specially.

One observation on Bharath's 0001 while I am here: I do not think it actually
conflicts with what you wrote. It applies the reloption when one is present,
and only reads pg_attribute when there is none, so "applying the value set in
the reloption at the moment of the dump" still holds under it. The two of you
differ over the reloption-absent case, where there is no value set to apply,
rather than over which of the two wins.

Attached is a much smaller thing, offered to make the discussion concrete
rather than as the answer. Report the loss at the point where it happens,
and leave pg_dump alone entirely.

=# ALTER TABLE t8 RESET (toast_value_type);
WARNING: TOAST relation of "t8" keeps "oid8" as the type of its "chunk_id"
DETAIL: The type in use is only read when the TOAST relation is created.
HINT: Set "toast_value_type" to "oid8" to have a dump and restore preserve it.

Only a reset naming toast_value_type reports; resetting other options on the
same relation stays quiet, as does a reset on a relation whose TOAST relation
uses oid, and the documented upgrade path of setting oid8 is untouched.

Worth noting that the tests you committed in c68cba09dd7 already reset the
option on toasttest_oid8, so the warning lands there and the expected output
records it, which is the whole change outside tablecmds.c.

If you would rather the dump carry the type, Bharath's 0001 is the better
patch and this one should be dropped. My preference is either of them over
the current behaviour. His 0002 on the migration docs seems worth having
either way, and independent as you say.

best.

-greg

Attachment Content-Type Size
0001-Warn-when-a-reset-of-toast_value_type-drops-oid8-fro.patch application/octet-stream 3.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Álvaro Herrera 2026-09-15 15:30:54 Re: REPACK (CONCURRENTLY) doesn't handle invalid indexes
Previous Message David Steele 2026-09-15 15:05:22 Re: Return pg_control from pg_backup_stop().