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

Hi,

On Mon, Sep 7, 2026 at 8:50 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> There is still something that I am wondering about this part of the
> patch, though, should we still publish in the output of
> pg_control_checkpoint() and pg_controldata an Oid extracted from the
> Oid8 value? I have not done that, and that does not make the control
> file larger. Just wondering if that would be useful to keep because
> we cannot really do an oid8->oid cast, and the oid information could
> still be useful on its own?

I don't think pg_control_checkpoint() and pg_controldata emitting an
OID extracted from the OID8 value is the right idea. I understand that
tools and external modules do need to adapt to the wider value, but
that is fine for a new major version, and the control file version
bump gives them the signal to do so. Also, the extra 4 bytes from OID8
are absorbed by existing padding, so the control file stays well under
the safe size limit of 512 bytes anyway.

> The rest of the patch set is mostly following the same organization,
> and I have spent a few hours working more on the last piece, making
> cleaner the contract with the vartag handling (one could not the
> changes in varatt.h, where I have changed to inline function for the
> get/set Oid8 values in varatt_external_oid8).
>
> 0005~0007 in the set of patches cover gaps in the regression tests.
> 0005 is the same one set of SQLs as the previous patch set (mostly),
> 0006 and 0007 cover more cases with test_decoding, amcheck,
> reloptions, the compression function, and the external toast pointer
> comparison in toast_tuple_init(). They have been split as they can
> still work once the reloption supports oid8, even if we don't make the
> varatt_external added yet (last patch).

Thanks for the v15 patches. I reviewed and tested (including pgindent)
the patches.

v15-0001-Enlarge-OID-generation-to-8-bytes.patch: this looks good to
me, with one comment. Although the wraparound with an 8-byte OID is
hundreds of years away, and 0001 assumes it never happens, do we need
to handle the case where someone sets the OID counter closer to the
8-byte OID limit using pg_resetwal and still hits the wraparound? We
can either reject this in pg_resetwal, handle the wraparound case in
the caller, document it, or mention it in the commit message or
somewhere near pg_resetwal. This is not a blocker for getting 0001 in,
just to be clear. I think this needs to be fixed as part of
v15-0006-Add-support-for-oid8-TOAST-values.patch, which does not check
for an existing TOAST OID.

v15-0002-Refactor-some-TOAST-value-ID-code-to-use-Oid8-in.patch: this
looks good to me. For the reorderbuffer.c change, I microbenchmarked
uint32_hash and tag_hash with a small C program that calls each 200M
times on the same key value, 4-byte for uint32_hash and 8-byte for
tag_hash (the existing 4-byte chunk_id case). The per-call cost for
both is 17 to 20 ns/call (run-to-run noise), and the difference is
only about 1 to 2 ns/call. This cost may well be negligible in the
midst of hashing, lookup logic and most importantly the overall
decoding costs. So, use of tag_hash looks fine to me here.

v15-0003-Switch-pg_column_toast_chunk_id-return-value-fro.patch: this
looks good to me, with one comment. The chunk_id cast in
cluster-toast-value-reuse.spec seems redundant, because that column
comes from the output of pg_column_toast_chunk_id(). In
misc_functions.sql it is needed. This makes me think about whether we
need to support OID to OID8 comparison in more cases: existing queries
and monitoring tools that do pg_column_toast_chunk_id(x) = some_oid,
or that store the result in an OID column, will now get a
type-mismatch error and have to add an explicit cast. Could we ease
that by making the OID to OID8 cast implicit, so that an OID compared
against an OID8 is promoted automatically? This is not a blocker for
getting 0003 in, just to be clear.

v15-0004-Add-support-for-TOAST-chunk_id-type-in-binary-up.patch: this
looks good to me, with a nit. Use
format_type_be(binary_upgrade_next_toast_chunk_id_typoid) instead of
%u for better readability in the "cannot support toast chunk_id type"
error message.

v15-0005-Add-relation-option-toast_value_type.patch: some comments.
1/ Parenthesize defaulttarg.
+/*
+ * RelationGetToastValueType
+ * Returns the relation's toast_value_type. Note multiple eval of argument!
+ */
+#define RelationGetToastValueType(relation, defaulttarg) \
+ ((relation)->rd_options ? \
+ ((StdRdOptions *) (relation)->rd_options)->toast_value_type : defaulttarg)

2/ Nit: double space after "creating".

+ <literal>chunk_id</literal> used when initially creating a toast
+ relation for this table.

3/ I understand that v15-0008 adds more tests, but can we have simple
tests in this patch as well, like creating a table with
toast_value_type='oid' and with an unknown type to verify the error?
This would make this patch independent.

4/ I checked that pg_dump emits the new value of this reloption even
when it is changed after the table was created. The existing TOAST
table and its index are not touched by the ALTER, but the new value is
stored in pg_class.reloptions, and that is what pg_dump emits. I think
this is fine, since dump and restore is the only way for a user to
migrate an existing table to an OID8 TOAST table.

5/ Following on from (4). Say I create a table with oid, insert data,
change the reloption to oid8, and then run a rewrite (VACUUM FULL,
CLUSTER or REPACK). The final table still uses oid, because the
rewrite preserves the existing TOAST table by swapping contents rather
than recreating it. But the transient table that make_new_heap()
builds does get created with the new type, because it goes through
create_toast_table(), which reads the reloption without checking
whether this is a rewrite. With oid this makes no difference, but once
oid8 is added the transient TOAST table would be oid8 while the
existing TOAST table still says oid. Is that expected?

I will get to reviewing v15-0006 to v15-0011 soon.

I verified the following cases on top of the v15 patch set.

1/ Measured the additional storage space and impact on query
performance for the same TOAST limits under 4 billion (OID limit).

At 4 GB TOAST (500k rows × 8 KB), oid8 costs +40% in TOAST index size
and +15% in heap size, with no measurable change to the TOAST table
size or full-detoast scan time. Queries [1]. The heap increase comes
from the on-disk TOAST pointer growing 4 bytes and the tuple
alignment. The TOAST index grows for the same reason on its chunk_id
key. The TOAST table size stays about the same because the extra
chunk_id bytes are small next to the roughly 2 KB of chunk_data and do
not change how many chunks fit per page.

table heap toast table toast index scan (3 reps, ms)
t_oid 25 MB 3935 MB 54 MB 3995 / 3971 / 3957
t_oid8 29 MB 3944 MB 75 MB 3996 / 3805 / 3809
delta +15.4% +0.23% +40.3% ~0 (within noise)

2/ Used oid8 for all tables by default and ran regression tests to see
if there are any issues.

make check seems happy with just one expected failure [2]. Fixing that
makes it run to the end successfully.

make check-world also seems happy with just one expected failure:
pg_amcheck 004_verify_heapam.pl fails [3] because it hardcodes a
4-byte OID TOAST pointer (HEAPTUPLE_PACK_LENGTH=58). oid8 widens the
pointer to 22 bytes, so the tuple is 62, not 58. Fixing that makes it
run to the end successfully. This makes me think we may need
pg_amcheck tests covering oid8.

I attached a diff file I used to make the tests happy on top of the
v15 patch set.

[1]
-- Server config: shared_buffers=2GB, maintenance_work_mem=1GB,
max_wal_size=8GB,
-- fsync=off, synchronous_commit=off, track_io_timing=on.
-- Release build (-O2), warm cache.

CREATE TABLE t_oid (id int, big bytea) WITH (toast_value_type='oid');
CREATE TABLE t_oid8 (id int, big bytea) WITH (toast_value_type='oid8');
ALTER TABLE t_oid ALTER COLUMN big SET STORAGE EXTERNAL;
ALTER TABLE t_oid8 ALTER COLUMN big SET STORAGE EXTERNAL;

INSERT INTO t_oid SELECT g, repeat('x',8000)::bytea FROM
generate_series(1,500000) g;
INSERT INTO t_oid8 SELECT g, repeat('x',8000)::bytea FROM
generate_series(1,500000) g;
VACUUM (ANALYZE) t_oid;
VACUUM (ANALYZE) t_oid8;

-- scan (forces full detoast through the toast index + table)
SELECT max(md5(big)) FROM t_oid;
SELECT max(md5(big)) FROM t_oid8;

[2]
# diff -U3 /local/home/rupiredd/postgres/src/test/regress/expected/psql.out
/local/home/rupiredd/postgres/src/test/regress/results/psql.out
# --- /local/home/rupiredd/postgres/src/test/regress/expected/psql.out
2026-08-20 01:24:10.447631698 +0000
# +++ /local/home/rupiredd/postgres/src/test/regress/results/psql.out
2026-09-08 23:00:17.613624770 +0000
# @@ -5193,7 +5193,7 @@
# TOAST table "pg_toast.pg_toast_2619"
# Column | Type
# ------------+---------
# - chunk_id | oid
# + chunk_id | oid8
# chunk_seq | integer
# chunk_data | bytea
# Owning table: "pg_catalog.pg_statistic"

[3]
doesn't match '(?^:(?^ms:heap table "postgres\.public\.test", block 0,
offset 5:\s+)data begins at offset 152 beyond the tuple length 58)'
# Looks like you failed 1 test of 32.
t/004_verify_heapam.pl ... Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/32 subtests

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com