I am mostly worried about performance degradation caused by the toast index not fitting in memory.
This is more likely to happen for huge tables, the case for which the 8-byte toast pointer is proposed. Have you compared the index sizes of OID and 8-byte ID ? It is possible, that they are of the same size because of alignment, so there would not be any difference in size. On Thu, Jul 30, 2026 at 10:01 AM Yugo Nagata <[email protected]> wrote: > On Thu, 30 Jul 2026 13:38:18 +0900 > Yugo Nagata <[email protected]> wrote: > > > On Fri, 8 May 2026 15:07:13 +0900 > > Michael Paquier <[email protected]> wrote: > > > > > Hi all, > > > > > > This is a follow-up of the previous thread about $subject, with a > > > reworked patch set for discussion in v20, as I care about the subject: > > > https://www.postgresql.org/message-id/[email protected] > > > > > > The main feedback of the previous thread is that the previous > > > implementation with its callbacks for each vartag was not liked much, > > > and their were concerns with pointer redirections and performance. > > > This patch set uses what I am calling the "brutal" approach, relying > > > on a vartag_external of a varlena or the atttype of the TOAST relation > > > to decide which external toast pointer we should use. This uses no > > > function pointers, and patches the code to deal with Oid or Oid8 TOAST > > > values where it matters. So, this time, performance cannot really be > > > an issue. > > > > > > The patch set is structured so as all the ground work happens first > > > (most of it comes from the previous patch set, reorganized a bit), and > > > the introduction of the varatt pieces are last, based on the following > > > rules: > > > - A table can use a 8-byte TOAST value with a new reloption, named > > > toast_value_type that can be set to "oid" (default) or "oid8", > > > creating a TOAST table with a value of the assigned type. This > > > includes support for dumps as well as binary upgrades, so as the > > > atttype of the chunk_id of the TOAST table is preserved. A table with > > > a TOAST type assigned cannot be changed to a different type through a > > > VACUUM FULL or a rewrite, as a matter of implementation simplicity. > > > - Renames and cleanup of various areas related to varatt_external, > > > renaming things to use OID. > > > - The 8-byte TOAST values rely on a Oid8, whose value is retrieved > > > from the control file extended by 4 bytes. The code supports > > > wraparound of values so as we don't assign anything between 0 and > > > FirstNormalObjectId for the lower bytes, same way as before. > > > > > > The last patch introduces a new vartag_external and the new > > > varatt_external_oid8, with an Oid8 as value. Well, not exactly, the > > > patch uses two uint32 fields so as the structure is packed without > > > padding, as of: > > > typedef struct varatt_external_oid8 > > > { > > > int32 va_rawsize; > > > uint32 va_extinfo; > > > uint32 va_valueid_lo; > > > uint32 va_valueid_hi; > > > Oid va_toastrelid; > > > } varatt_external_oid8; > > > > > > Note that if applying all the patches except the last one, the code > > > would use an varatt_external_oid with an oid8 TOAST table. This > > > works, the split is to make reviews easier. An oid8 TOAST table > > > always uses a varatt_external_oid8. > > > > > > I have done a lot of back-and-forth in the patch to try to find a good > > > balance between the manipulation of the varlenas in the detoast and > > > compression paths, as well as reorderbuffer.c and amcheck. And I have > > > finished with the attached, which is kind of nice. The last patch has > > > a low footprint: > > > 9 files changed, 537 insertions(+), 208 deletions(-) > > > > I applied the patches to the master branch and run a simple test. > > > > After running: > > > > $ pg_resetwal -o 4300000000 -D data > > > > CREATE TABLE tbl_oid8(t text) with (toast_value_type = 'oid8'); > > ALTER TABLE tbl_oid8 ALTER t SET STORAGE EXTERNAL; > > CREATE TABLE tbl_oid(t text) with (toast_value_type = 'oid'); > > ALTER TABLE tbl_oid ALTER t SET STORAGE EXTERNAL; > > > > INSERT INTO tbl_oid8 VALUES (repeat(md5('x'),100)); > > INSERT INTO tbl_oid VALUES (repeat(md5('x'),100)); > > > > SELECT tableoid, t = repeat(md5('x'),100) FROM tbl_oid8; > > tableoid | ?column? > > ----------+---------- > > 5032704 | t > > (1 row) > > > > SELECT chunk_id, chunk_seq FROM pg_toast.pg_toast_5032704; > > chunk_id | chunk_seq > > ------------+----------- > > 4300000010 | 0 > > 4300000010 | 1 > > (2 rows) > > > > SELECT tableoid, t = repeat(md5('x'),100) FROM tbl_oid; > > tableoid | ?column? > > ----------+---------- > > 5032709 | t > > (1 row) > > > > SELECT chunk_id, chunk_seq FROM pg_toast.pg_toast_5032709; > > chunk_id | chunk_seq > > ----------+----------- > > 5032715 | 0 > > 5032715 | 1 > > (2 rows) > > > > This appears to work as expected. > > I also run a simple performance test. I measured the TPS on > the master branch and with the current patches applied under the > following conditions: > > Prepare: > drop table if exists tbl; > create table tbl (id int primary key, j int, t text); > alter table tbl alter column t set storage external; > insert into tbl select i, i, repeat(md5(i::text),100) > from generate_series(1,10000) i; > > query.sql: > \set id random(1, 10000) > update tbl set (j,t) = (j + 1, repeat(md5((j+1)::text),100)) > where id = :id; > > Running pbench three times: > > $ pgbench test -f query.sql -c 8 -j 4 -T 100 > > Results: > > master: 2686.229104, 2697.655096, 2687.894343 > patched: 2685.292256, 2692.080214, 2690.005753 > > I could not observe any performance degradation with the curernt patches, > at least in my environment. > > I haven't tested whether the previous version of the patches showed the > performance degradation that has been a concern, though. > > > > > I noticed one issue, though. > > When toast_value_type is specified as something other than oid or oid8, > the error > > message is: > > > > postgres=# CREATE TABLE tbl(t text) with (toast_value_type = 'x'); > > ERROR: invalid value for enum option "toast_value_type": x > > DETAIL: Valid values are "oid". > > > > The detail message should be updated to include oid8. > > > > > > I also have a few small comments on the patches. > > > > - 0009 > > > > (1) > > The commit message says: > > > > TOAST pointers still rely on varatt_external and a single vartag, with > > all the values inserted in the bigint TOAST tables fed from the existing > > OID value generator. > > > > This made me a bit confused because chunk_ids insereted into TOAST > tables using oid8 > > are generated by GetNewObjectId8(). > > > > + else if (toast_typid == OID8OID) > > + toast_pointer.va_valueid = GetNewObjectId8(); > > > > This value is later cast to Oid here, so what is actually stored is a > 4-byte chunk_id, > > but saying that the values are "fed from the existing OID value > generator" seems > > a bit inaccurate to me. > > > > (2) > > /* > > - * Choose an OID to use as the value ID for this toast value. > > + * Choose a new value to use as the value ID for this toast value, > be it > > + * for OID or int8-based TOAST relations. > > > > This patch relies on oid8, introduced by b139bd3b6ef, rather than int8, > so, I > > wonder if this comment sould also be updated. > > > > (3) > > belonging to the owning table. Every > > -<acronym>TOAST</acronym> table has the columns > <structfield>chunk_id</structfield> (an OID > > -identifying the particular <acronym>TOAST</acronym>ed value), > > +<acronym>TOAST</acronym> table has the columns > > +<structfield>chunk_id</structfield> (an OID or an 8-byte integer > identifying > > +the particular <acronym>TOAST</acronym>ed value), > > <structfield>chunk_seq</structfield> (a sequence number for the chunk > within its value), > > > > Similarly, I think the documentation should mention oid8 rather than > 8-byte > > integer. That would also be consistent with the CREATE TABLE > documentation for > > the toast_value_type parameter. > > > > - 0011 > > > > (4) > > + /* > > + * Check if this value already > exists in the new toast > > + * table (corner case during table > rewrite with multiple > > + * versions of the same row). > > + */ > > + if > (toastrel_valueid_exists(toastrel, va_valueid)) > > + { > > + /* Match, so short-circuit > the data storage loop below */ > > + data_todo = 0; > > + } > > > > The same code appears later, together with a more detailed comment. How > about > > moving the detailed comment here instead, or simply referring to the > later comment? > > > > (5) > > +/* Is varlena datum a pointer to on-disk toasted data with 8-byte value > ID? */ > > +static inline bool > > +VARATT_IS_EXTERNAL_ONDISK_OID8(const void *PTR) > > +{ > > + return VARATT_IS_EXTERNAL(PTR) && VARTAG_EXTERNAL(PTR) == > VARTAG_ONDISK_OID8; > > } > > > > This macro is defined but not used. Is it intended to be used in places > that > > check VARTAG_ONDISK_OID8 directly, for example: > > > > + if (VARTAG_EXTERNAL(attr) == VARTAG_ONDISK_OID8) > > > > ? > > > > (6) > > > > any. Allowing for the varlena header bytes, > > -the total size of an on-disk <acronym>TOAST</acronym> pointer datum is > therefore 18 > > -bytes regardless of the actual size of the represented value. > > +the total size of an on-disk <acronym>TOAST</acronym> pointer datum is > 18 > > +bytes when using an OID as <structfield>chunk_id</structfield>, or 22 > bytes > > +when using an 8-byte integer, regardless of the actual size of the > represented > > +value. > > </para> > > > > Should this also refer to Oid8 rather than 8-byte integner, for > consistency? > > I have one additional, very trivial comment: > > (7) > > /* > - * Open the toast relation and its indexes > + * Determine the pointer type from the datum's vartag and extract > the > + * toast relation OID and value ID accordingly. The vartag tells > us > + * everything we need ― no TOAST table schema lookup required. > > Non-ASCII hyphen is used in this comment. This may not be prohibitted, > but I don't think it's common in the PostgreSQL code base. > > Regards, > Yugo Nagata > > -- > Yugo Nagata <[email protected]> > > >
