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

On Wed, Sep 02, 2026 at 01:27:00PM -0700, Bharath Rupireddy wrote:
> I first tried to reproduce the query latency increase problem with
> less time and disk space. Here are the results. I ensured each insert
> steps over K ids using pg_resetwal, which equals the number of
> iterations spent in GetNewOidWithIndex(). There's a clear benefit as
> the 8-byte chunk_id avoids the retries while getting the new OID, and
> since a 64-bit OID is almost never exhausted, this works. Attached the
> test script that I used for reference.
>
> K HEAD_INSERT patched_INSERT
> 1,000,000 1.25 s 1.0 ms
> 10,000,000 12.4 s 1.0 ms
> 50,000,000 1 min 1.0 ms
> 100,000,000 2 min 1.0 ms
> 200,000,000 9 min 0.9 ms

This measures how much time we take to grab a free OID value, hoping
that there is a hole.

> With this context, I started to review the patches. One thing I liked
> is the way the patches were split; they made review a lot easier. Here
> are some comments.

Thanks for the comments.

> Comments on 0001:
>
> 1/
> +table_relation_fetch_toast_slice(Relation toastrel, Oid8 valueid,
> While changing Oid to Oid8 is fine for internal functions, is it okay
> to change it for the table_relation_fetch_toast_slice table AM? Is
> there any chance that an external table AM forgets to update? I'm not
> arguing against this change, just checking if there's any way out
> here. Of course, the compiler will generate an incompatible function
> parameter type warning, and since the change is going in a major
> release, that seems fine. One idea could be to have a TOAST version
> field elsewhere to detect such changes, but that seems overkill. So,
> having it like 0001 seems fine to me unless others have any thoughts.
> PS: one option is to send the 8-byte OID via a caller-allocated
> varlena result pointer, but this seems a bit ugly.

Yes, assuming that a table AM uses its own fetch_toast_slice(), it
will need to update probably for its own ScanKeyInit() if an external
TOAST table is in use for the OID8 case, assuming that they need to do
so based on the reloption for the toast value type. I am not really
convinced that we need to be fancy here, telling that after looking at
out-of-code projets that include their own callbacks:
https://github.com/eatonphil/pgtam (no support)
https://github.com/rohankumardubey/pg_mooncake
https://github.com/neurdb/neurdb (no support)
https://github.com/duckdb/pg_duckdb (totally different callback here)
https://github.com/timescale/timescaledb
https://github.com/jeffreydwalter/pg_tde/ (needs refresh)
https://github.com/ewhauser/unitpg/ (just reuses the heap call)

There are also a few more, but I am seeing nothing beyond an extra
ScanKeyInit() update required, in a sea of non-supported cases.

> 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.

Yeah, point taken. I'm putting a note down on this one.

> I think 0002 and 0003 are purely mechanical; they look good to me and
> can go in first.

Thanks. 0002 and 0003 are kind of independent on the rest, not
requiring any Oid8 idea, so I'll go apply them.

> 1/
> - WHERE o.chunk_id != pg_column_toast_chunk_id(c.value); + WHERE
> o.chunk_id::oid8 != pg_column_toast_chunk_id(c.value);
>
> Changing the return value of pg_column_toast_chunk_id() to 8-byte OID
> is fine and it can work for existing 4-byte chunk_ids. Do we need the
> above typecasting in tests given the chunk_id is captured from
> pg_column_toast_chunk_id() while creating the table?

I guess we do. Will double-check.

> Comments on 0005:
>
> 1/ + return murmurhash64(DatumGetObjectId8(datum));
>
> I think having catalog cache support for 8-byte OID might be useful on
> its own and it looks good to me except the following: can we typecast
> the Oid8 to murmurhash64((uint64) DatumGetObjectId8(datum))?

Ah, you want to add an extra cast here, with (uint64) for the
murmurhash64. Hmm why not, that seems more consistent with the rest
of the area. This one is also a rather independent item for the Oid8,
so I'm also planning to apply it to reduce the stack.

> Comments on 0008:
>
> 1/ I think adding reloption seems okay to me, so the 0008 patch looks
> good. Just one suggestion: having a GUC to set it once for all new
> tables seems a good idea as it avoids application changes, but
> starting with reloption is good enough.

The round one of the patch set used a GUC. Andres has argued in favor
of a reloption.

> I will take a look at 0006, 0007, 0009, 0010, 0011 in the coming weeks.

Cool, thanks.

> One more thought: existing tables won't get 8-byte chunk_ids as part
> of pg_upgrade. pg_dump and pg_restore could be used after the upgrade
> for existing customers to get their tables onto this format. At some
> point, providing concurrent repack-like support (or reusing the
> underlying machinery) to do this online would be nice to have.

The infrastructure that could be used to switch the toast value type
and its rewrite is out of scope, the patch being complicated enough..
We could always think about that later, giving the choice in core is
much more important to me as a first step, because without the
OID8/OID choice, there is no discussion about the rewrite part.
--
Michael