pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Greg Burd <greg@burd.me>
To: Michael Paquier <michael@paquier.xyz>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Yugo Nagata <nagata@sraoss.co.jp>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Support for 8-byte TOAST values, round two
Date: Tue, 15 Sep 2026 11:26:11 -0400
Message-ID: <5c7124c4-c481-403c-b02d-cc80eaae94ef@app.fastmail.com> (raw)
In-Reply-To: <aqju1K96ex28yz2X@paquier.xyz>
References: <4272005a-ca62-46be-aab3-8678187d84cd@app.fastmail.com>
	<e971c02b-3472-4f94-9259-a0c70675206e@app.fastmail.com>
	<aqHpYL-ytLbT3jGR@paquier.xyz>
	<aqJhmsEEiAUKQFXN@paquier.xyz>
	<CALj2ACWEY0HHREE5Dn9Waa33coj-SwxPRkEtW4A=nCSzN8UrAQ@mail.gmail.com>
	<CALj2ACVG9cjxr-0jb8PP_Vb7vJvVGBVM_fQ2srt4CpyURpfYhQ@mail.gmail.com>
	<aqOu0TEpCM_oPksP@paquier.xyz>
	<CALj2ACVCicT3mWFD4Y-QsR+67THZ8J3FPmFtpTns2JxG3udGUQ@mail.gmail.com>
	<aqd_gYs35v--sVlR@paquier.xyz>
	<CALj2ACVLbSB1L+ObOhXE_ZS3ZVu50Cm-BsM5AGcc4grnO_F5JQ@mail.gmail.com>
	<aqju1K96ex28yz2X@paquier.xyz>


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

Attachments:

0001-Warn-when-a-reset-of-toast_value_type-drops-oid8-fro.patch (application/octet-stream, 3.8K) download From 63fb0ea13e86760ab0460bf7f3dd165baf32032d Mon Sep 17 00:00:00 2001 From: Greg Burd <greg@burd.me> Date: Tue, 15 Sep 2026 07:46:39 -0400 Subject: [PATCH] Warn when a reset of toast_value_type drops oid8 from a dump toast_value_type is read only when a TOAST relation is created, so a reset leaves the TOAST relation alone: a relation created with oid8 keeps it, and pg_attribute becomes the only place recording the type in use. Nothing fails, and the chunk identifiers are assigned afresh on a restore, but the dump carries no trace of the type that was dropped and the relation comes back limited to 2^32 out-of-line values. That matters because restoring an oid8 relation holding more than four billion out-of-line values into an oid TOAST relation spins in GetNewOidWithIndex(), so the first sign of trouble can be a restore that does not finish, on exactly the relations big enough to have wanted oid8. Report the loss where it happens rather than leaving it to be found on the restore. Only a reset naming toast_value_type is reported, so resetting other options on the same relation stays quiet, as does a reset on a relation whose TOAST relation uses oid. This is deliberately the smallest change that removes the silence; it does not touch what pg_dump emits. Discussion: https://postgr.es/m/aqj6yd7ztQldv4Tu@paquier.xyz --- src/backend/commands/tablecmds.c | 30 +++++++++++++++++++++++++++ src/test/regress/expected/strings.out | 3 +++ 2 files changed, 33 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 2f073ddb84a..ba7e0931099 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -17355,6 +17355,36 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, newOptions = transformRelOptions(datum, defList, NULL, validnsps, false, operation == AT_ResetRelOptions); + /* + * A reset of toast_value_type leaves the TOAST relation alone, so a + * relation created with oid8 keeps it, and pg_attribute becomes the only + * place recording the type in use. Nothing fails, but a later dump and + * restore recreates the TOAST relation from the reloptions and so hands + * the relation back limited to 2^32 out-of-line values. Say so, since + * the loss is otherwise silent and only shows up on the restore. + */ + if (operation == AT_ResetRelOptions && + RelationGetToastChunkIdType(rel) == OID8OID) + { + ListCell *cell; + + foreach(cell, defList) + { + DefElem *def = (DefElem *) lfirst(cell); + + if (def->defnamespace != NULL || + strcmp(def->defname, "toast_value_type") != 0) + continue; + + ereport(WARNING, + (errmsg("TOAST relation of \"%s\" keeps \"oid8\" as the type of its \"chunk_id\"", + RelationGetRelationName(rel)), + errdetail("The type in use is only read when the TOAST relation is created."), + errhint("Set \"toast_value_type\" to \"oid8\" to have a dump and restore preserve it."))); + break; + } + } + /* Validate */ switch (rel->rd_rel->relkind) { diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out index fa29abfd829..902a3671e28 100644 --- a/src/test/regress/expected/strings.out +++ b/src/test/regress/expected/strings.out @@ -2304,6 +2304,9 @@ SELECT c1.relname, a.atttypid::regtype ALTER TABLE toasttest_oid RESET (toast_value_type); ALTER TABLE toasttest_oid8 RESET (toast_value_type); +WARNING: TOAST relation of "toasttest_oid8" 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. -- Reset column storage to its default ALTER TABLE toasttest_oid ALTER COLUMN f1 SET STORAGE EXTENDED; ALTER TABLE toasttest_oid8 ALTER COLUMN f1 SET STORAGE EXTENDED; -- 2.50.1 (Apple Git-155)

view thread (72+ messages)  latest in thread

Message-ID: <5c7124c4-c481-403c-b02d-cc80eaae94ef@app.fastmail.com>
Permalink:  ../5c7124c4-c481-403c-b02d-cc80eaae94ef@app.fastmail.com/
Also on:    postgresql.org/message-id/5c7124c4-c481-403c-b02d-cc80eaae94ef@app.fastmail.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: greg@burd.me, michael@paquier.xyz, bharath.rupireddyforpostgres@gmail.com, nagata@sraoss.co.jp, pgsql-hackers@lists.postgresql.org
  Subject: Re: Support for 8-byte TOAST values, round two
  In-Reply-To: <5c7124c4-c481-403c-b02d-cc80eaae94ef@app.fastmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox