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: Mon, 14 Sep 2026 10:01:15 -0400
Message-ID: <c4fee437-ea3d-446c-84dd-5e105afa0b2c@app.fastmail.com> (raw)
In-Reply-To: <aqd_gYs35v--sVlR@paquier.xyz>
References: <CALj2ACWTM=VgjDU=2BBuYAh2Ae8Kq5NspJZsmhiAvNQkXZ=MjQ@mail.gmail.com>
	<aqCvLyFwAwviycfu@paquier.xyz>
	<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>


On Mon, Sep 14, 2026, at 1:00 AM, Michael Paquier wrote:
> On Fri, Sep 11, 2026 at 08:44:26AM -0700, Bharath Rupireddy wrote:
>> I reviewed v17 patches, overall they look good to me.
>> 
>> v17-0001-Refactor-some-TOAST-value-ID-code-to-use-Oid8-in.patch: It
>> v17-0002-Switch-pg_column_toast_chunk_id-return-value-fro.patch: It
>
> I have discarded the change of reorderbuffer.c that was in v17-0001,
> and added it back to what's now v18-0001.  Then applied these two
> after an extra round of self-review.
>
>> v17-0003-Add-support-for-oid8-TOAST-values.patch: It looks good to me
>> with one nit. Since chunk_id is always at attnum = 1 and all the
>> toast_valueid_scankey_init() callers pass it as 1, and this init
>> function is just to fetch the chunk_id, can we just hard-code it
>> inside and remove the attnum function parameter?
>
> If one wants to control which attribute number to look after, that
> could be extended more later on, yes.  I have removed the argument
> from the function in v18-0001.
>
>> v17-0004-Add-battery-of-tests-related-oid8.patch: It looks good to me.
>
> Okay, thanks.
>
>> pg_dump/pg_restore for demoing OID4 to OID8 migration for
>> existing tables and TOAST table tests during pg_upgrade could be
>> follow-up patches. This makes me think, if someone does an OID8 to
>> OID4 migration and the chunk_ids are beyond the 4-billion limit, the
>> restore should fail rather than silently wrapping the chunk_ids and
>> causing TOAST index insert failures. My point is, we need to test this
>> case as well.
>
> Nah, I don't buy that this is worth the cycles.  If one has the idea
> to create an oid8 TOAST table, inserts more than 4 billion values into
> it, dumps and restores into a table that uses a oid TOAST then the
> result is an infinite loop in the backend on restored INSERTs.
>
> Something that I still see as a hole is that we have the tables from
> the strings test in the main regression test suite and we could
> exploit that a bit more.  Hmm.  How about resetting the OID in the
> control file earlier in the pg_upgrade test and check with a SELECT
> post-upgrade that the counters generated by the main regression test
> suite are more than 2^32?
>
> Added also some tests for REPACK here in what's now 0002, based on the
> feedback of upthread, to check that the a REPACK's rewrite does not
> touch the TOAST table definition.
>
>> v17-0005-Add-support-for-TOAST-pointers-as-oid8.patch: It looks good to me.
>
> And rebased is a v18 with the three remaining pieces, plus two more
> test patches: the tests of Rui with decoding and oid8 values higher
> than 2^32, and my test to have the main regression test suite of
> pg_upgrade run with a default oid8 in the control file higher than
> 2^32, checking that assigned chunk numbers are handled correctly.
> --
> Michael
>
> Attachments:
> * v18-0001-Add-support-for-oid8-TOAST-values.patch
> * v18-0002-Add-battery-of-tests-related-oid8.patch
> * v18-0003-Add-support-for-TOAST-pointers-as-oid8.patch
> * v18-0004-Decode-an-oid8-TOAST-value-with-an-ID-past-2-32.patch
> * v18-0005-Add-check-for-2-32-oid8-chunk-values-across-the-.patch
> * signature.asc

Hello Michael, all,

I tried v18 out, as expected patches/tests/etc. all good on my macOS/arm64
laptop.

I agree with Bharath that dropping the attnum argument from
toast_valueid_scankey_init() was right, and I see it is gone in 0001.

I like what you did in 0005 with the test, moving the pg_resetwal call ahead
of the main regression suite means the whole suite now runs with the counter
already past 2^32, rather than just one case checking a large value.

  ok 1 - set an 8-byte OID counter in the old instance
  ok 7 - oid8 chunk_ids are past 2^32
  ok 25 - 8-byte OID counter is carried over by pg_upgrade

I'd say that closes the gap Bharath identified, where every oid8 test still
produced chunk_ids that fit in four bytes. Rui's decoding test in 0004 covers
the other half, and I checked separately that it fails when it should: with
ReorderBufferToastAppendChunk() put back to the truncating 4-byte read it
reports

  not ok 23 - oid8 TOAST value past 2^32 is decoded in full
  # table public.toasted_oid8: INSERT: id[integer]:1 data[text]:unchanged-toast-datum

and passes again once restored.

The REPACK additions in 0002 match what I measured by hand: with the reloption
flipped, VACUUM FULL, CLUSTER and REPACK all leave the TOAST table's chunk_id
type alone.

A quick check of my own on top of v18, counter preset to 5000000000: chunk_ids
5000000005 through 5000000054, all past 2^32, data intact, verify_heapam()
reports nothing on either the TOAST relation or the main table, and REPACK
leaves chunk_id as oid8 with the data still intact.

Nothing blocking from me on v18.

best.

-greg






view thread (72+ messages)  latest in thread

Message-ID: <c4fee437-ea3d-446c-84dd-5e105afa0b2c@app.fastmail.com>
Permalink:  ../c4fee437-ea3d-446c-84dd-5e105afa0b2c@app.fastmail.com/
Also on:    postgresql.org/message-id/c4fee437-ea3d-446c-84dd-5e105afa0b2c@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: <c4fee437-ea3d-446c-84dd-5e105afa0b2c@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