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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Support for 8-byte TOAST values, round two
Date: 2026-09-07 07:54:35
Message-ID: ap5tu4VkJgsEthVq@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Sep 04, 2026 at 08:11:00PM -0700, Bharath Rupireddy wrote:
> On Wed, Sep 2, 2026 at 9:58 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> On Wed, Sep 02, 2026 at 01:27:00PM -0700, Bharath Rupireddy wrote:
>>> Widening the per-toast-chunk hash key to an 8-byte OID still handles
>>> 4-byte chunk_ids correctly during logical decoding, since the smaller
>>> values zero-extend. The tradeoff is that with an 8-byte key the hash
>>> no longer uses the uint32_hash fast path and falls back to tag_hash,
>>> so even existing 4-byte chunk_id tables lose that optimization during
>>> logical decoding. I haven't measured the effect, but it seems worth
>>> checking.

While navigating through the code, I am not planning to worry about
it at the end, as the performance sits close to some WAL record
handling and tuple copying, where most of the performance is on the
table there.

> Also, I think there's an issue in ReorderBufferToastAppendChunk() with
> 0001. The chunk_id is still fetched as a 4-byte value, so past 4
> billion chunk_ids the truncated key stored in the TOAST hash won't
> match the 64-bit value used for the lookup later in
> ReorderBufferToastReplace().

Hmm. This one may be debatable because it would not be reached until
the OID8 business is plugged into the TOAST values. Adding that in
0001 is less business to deal with after. For now I've fixed that in
0001 to keep the changes with reorderbuffer.c more isolated.

> That works for me. A quick question, though I haven't tested this.
> Does ALTER TABLE error out if the reloption is changed on an existing
> table?

I was not doing that with only simplicity in mind, so one could change
toast_value_type after the initial TOAST table creation, with one
argument in mind but I agree that it may sounds a bit weird: dumps.

One could change the reloption, do a dump, and re-create the relation
with a different TOAST layer. Agreed that it's a bit twisted but we
don't have a lot of precedent for this kind of choices with
reloptions. I recall one similar case in the backend code, with BRIN
indexes and pages_per_range (?) that do not affect post-creation
patterns. The WITH added by the dump is kind of nice for re-creation
based on raw data.

There is also a second scenario where this can be useful:
CREATE TABLE t (a int);
ALTER TABLE t SET (toast_value_type = oid8);
ALTER TABLE t ADD COLUMN b text; -- creates TOAST relation

I've mentioned that in the docs already, as of the "creating":
+ The toast_value_type specifies the attribute type of
+ <literal>chunk_id</literal> used when initially creating a toast
+ relation for this table

I have added some tests for ALTER TABLE with some rewrites, though.
That felt like a hole.

> That's fine by me. That said, it would be good to document at least
> one clear way for existing users to move their tables to 8-byte TOAST
> chunk_ids, presumably pg_dump and pg_restore after setting the
> reloption.

Hmm, yeah. Perhaps. The dump/restore trick is one, at least. Not
sure where adding this mention would be best, and if we actually need
to add that. :)

> I'm also wondering if pg_dump needs an option to generate table
> schemas with the new reloption set, so that users don't have to edit
> the dump file. Editing may not always be possible (for example, with
> the custom and directory formats). This can be a follow-up patch, but
> having at least one supported way to migrate existing tables seems
> important to me.

The best thing that I came up for that was a GUC, because you'd want
a dump/restore option able to say "I don't want the TOAST type" to be
dumped or restored, able to enforce a type across the board for a
database, a namespace or more than N objects at the same time. The
GUC idea was shut down in round one by Andres.

> Agreed, dropping it seems right at a quick glance. The chunk_id comes
> from the heap row's TOAST pointer and is used as a scan key against
> the TOAST index, so nothing looks up an OID8 value through the catalog
> cache.

Following c5806f2165b5 and 2b3d11aaed89, attached is a rebase of the
rest.

While on it, I have added one StaticAssertDecl() to enforce the
no-padding rule of vartag_external_oid8. There was also a spot I have
missed in pg_column_toast_chunk_id() to handle oid8 toast pointers, in
the last patch.

On top of that, while doing more review, there was also an issue that
I got on my notes for some time but completely forgot about in this
thread: toast_tuple_find_biggest_attribute() needs some relcaching for
the toast value type to enforce a correct decision, or we may be off
in deciding if something should be compressed or not depending on the
toast table we are dealing with. This uses the OID's max chunk size
as a safety measure, which should be OK. This part is added on top of
the rest, as 0009.

Finally, I have asked Claude for a round of reviews for the full patch
set, and it has spotted two stupid mistakes caused by the new vartag
for oid8, where I missed its handling:
- toast_get_compression_id() missed handling for oid8 external
pointers.
- toast_tuple_init() can compare two external TOAST pointers, but we
cannot memcmp() safely when dealing with tuples with different
vartags, the oid8 one being larger than the oid one. That's a nice
catch, for an egde case when doing cross-updates and TOAST pointer
reuse.

Attached is v14, with all that addressed (should perhaps have moved
the tests in 0008 to 0007, but it's late here and that does not change
the stuff to review).
--
Michael

Attachment Content-Type Size
v14-0001-Refactor-some-TOAST-value-ID-code-to-use-Oid8-in.patch text/plain 16.1 KB
v14-0002-Switch-pg_column_toast_chunk_id-return-value-fro.patch text/plain 5.7 KB
v14-0003-Add-support-for-TOAST-chunk_id-type-in-binary-up.patch text/plain 9.3 KB
v14-0004-Enlarge-OID-generation-to-8-bytes.patch text/plain 14.5 KB
v14-0005-Add-relation-option-toast_value_type.patch text/plain 6.4 KB
v14-0006-Add-support-for-oid8-TOAST-values.patch text/plain 17.1 KB
v14-0007-Add-tests-for-TOAST-relations-with-oid8-as-value.patch text/plain 23.0 KB
v14-0008-Add-support-for-TOAST-pointers-as-oid8.patch text/plain 54.0 KB
v14-0009-Fix-toast_tuple_find_biggest_attribute-for-OID8-.patch text/plain 6.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2026-09-07 08:10:52 RE: Follow-up review items for update_deleted
Previous Message Zhijie Hou (Fujitsu) 2026-09-07 07:52:59 RE: Follow-up review items for update_deleted