| From: | Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> |
|---|---|
| To: | Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> |
| Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Support for 8-byte TOAST values, round two |
| Date: | 2026-07-30 08:01:02 |
| Message-ID: | 20260730170102.15b4aded1698dee3c2a304e2@sraoss.co.jp |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, 30 Jul 2026 13:38:18 +0900
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
> On Fri, 8 May 2026 15:07:13 +0900
> Michael Paquier <michael(at)paquier(dot)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(at)sraoss(dot)co(dot)jp>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hannu Krosing | 2026-07-30 08:11:41 | Re: Direct Toast PoC |
| Previous Message | Daniel Gustafsson | 2026-07-30 07:59:24 | Re: datachecksums: handle invalid and dropped databases during enable |