Re: Support for 8-byte TOAST values, round two - Mailing list pgsql-hackers

On Thu, 30 Jul 2026 13:38:18 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

> On Fri, 8 May 2026 15:07:13 +0900
> Michael Paquier <michael@paquier.xyz> 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/aFOnKHG7Wn-Srnpv@paquier.xyz
> >
> > 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 <nagata@sraoss.co.jp>