Re: Direct TOAST v2, faster, smaller and no migration needed

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Hannu Krosing <hannuk(at)google(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Dilip Kumar <dilipkumarb(at)google(dot)com>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
Subject: Re: Direct TOAST v2, faster, smaller and no migration needed
Date: 2026-09-07 03:13:44
Message-ID: ap4r6H0A-5SD8pGj@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Sep 05, 2026 at 02:24:50PM +0200, Hannu Krosing wrote:
> Attached is a v2 patch series implementing "Direct TOAST", a new storage format
> for out-of-line (TOASTed) variable-length attributes in PostgreSQL.

Thanks for splitting that into a new thread.

> Michael Re: your concern in earlier discussion about just adding the
> direct toast checks directly into code next to
> VARATT_IS_EXTERNAL_ONDISK - this is doen this way because I consider
> direct toast to be a simplified and streamlined subtype of traditional
> toast which just cuts out the index lookup part. This is also
> exemplified by zero-downtime / zero-migration switch to direct toast
> (and back)

Noted. Now they are as well some arguments that come into mind that
don't make it sound as an acceptable design, because this proposal is
about tradeoffs, mostly. As far as I know, there is never a magical
solution when it comes to software, and restrictions of your patch set
are in place, reflecting these tradeoffs.

There's a bit of bloat in this message; quoting the most relevant
parts only to make that readable. There is a lot of AI bloat in your
text, perhaps reconsider this approach before posting to the lists...
But well..

This gist of the proposal can be summarized based on this, in simpler
words (not everything, but these are the most relevant pieces here):
- Add a new varatt_direct, that acts as a new type of external
pointer, replace va_valueid by a ItemPointerData.
- The va_tid points to a a chunk in the TOAST table, that includes an
array of tids. This array of tids redirects to each chunk.
- Instead of an index lookup combined to a heap lookup, we need to
retrieve two heap blocks, one to get the tids array, one for the chunk
itself.
- The array of tids is stored in the *last* chunk.
- Bypass the index handling, because the tids don't require that.
- Avoid the 4-byte OID value wraparound by design, as this switches to
a tid.

> Then I ran separate 1 hour runs of updates on top of same tables first
> for traditional toast then for direct toast .
>
> The direct toast did 1385 TPS while traditional toast did 635 TPS

So, in terms of benchmarks, this is a claim based on:
- a pgbench workload with many toasted atttributes.
- pgvector
- no mention of configuration, as far as I can see, or anything?
Putting this last point aside for a minute..

Claiming that this approach is simply "better" based on only a subset
of workloads is debatable, and how it could be better in some other
cases while not impacting the performance of the default 4-byte TOAST
OID? A few things that come on top of my mind:
- How does this fart with readahead? The tids array is in the last
block, if we have a cold cache and need to retrieve the last block
*before* looking at a block away from that, isn't that a penalty in
itself if data does not fit completely into OS cache or even
shared_buffers?
- Support for read of slices, where retrieving short cuts of the TOAST
data could pay the price due to the last chunk requirement. substr()
is a common thing for applications.
- Lock contention and concurrency. A btree page for a TOAST table in
cache is able to hold hundreds of references to various entries.
Claiming that this can be always outperformed is unclear, to say the
least, by switching to one array of tids for each value stored in
TOAST divided in chunks? I think that this puts more pressure on the
OS cache or PG shared buffers when dealing with many hot values.

> Crucially:
> sizeof(varatt_direct) == sizeof(varatt_external) == 18 bytes

This claim looks incorrect to me, I am quickly measuring:
sizeof(varatt_external) = 16
sizeof(varatt_direct) = 20

So, yeah, I'm also puzzled with this statement.

> 2.4. Lock-Free In-Place Schema Upgrades
> Existing tables can be upgraded from Plain to Direct TOAST on the fly:
> ALTER TABLE my_table SET (toast_flavour = 'direct');
> or via pg_ensure_direct_toast(reloid).
> This performs a metadata-only catalog update adding chunk_tids and
> chunk_tid_offsets with fast-default NULLs. No data rewrite or exclusive table
> lock is required. Plain and Direct TOAST datums can coexist within the same
> table indefinitely.

In v2-0002 (with comment pieces added to v2-0008 much later, no idea
why but):
+ TupleDescInitEntry(tupdesc, (AttrNumber) 4,
+ "chunk_tids",
+ TIDARRAYOID,
+ -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 5,
+ "chunk_tid_offsets",
+ INT8ARRAYOID,
+ -1, 0);

This is ensured by adding two new concepts to TOAST tables, even in
the existing TOAST 4-byte case: two new attributes and a partial
index. The existing attribute layer is moot when using one (4-byte
value) or the other (direct). This is a waste, and unlikely free.
The addition of a new partial index does not help much in that. I
understand that you've written that this way to claim a cheap rewrite
when switching over by manipulating data later on on upgrades, but
that does not sound acceptable here.

Finally, and the biggest elephant in the room here by far.. VACUUM
FULL, CLUSTER and REPACK *have* to be forbidden, because on rewrite
each command rewrites the tids in the parent. That's a legal
defensive set of commands because it is possible to reclaim bloat from
TOAST relations directly, and I doubt that we'd *ever* want to drop
this property, especially based on the benchmark claim of upthread. A
worst thing to me is that this seems to entirely disable their use due
to this in v2-0004, cluster_rel() or cluster.c:
+ if (OldHeap->rd_rel->relkind == RELKIND_TOASTVALUE)
[..,]
+ if (OldHeap->rd_att->natts >= 4 &&

The two new attributes are added *unconditionally*.

Another thing that is really disturbing to me is that using tids
lowers the protection regarding TOAST lookups. A TOAST value acts a
second barrier of protection if we miss a chunk, and we have a long
history of bugs in this area (spoiler: we still had two recent
discussions about the same set of issues for very old problems, still
unresolved). Relying on only a get_toast_snapshot() and a bare TID
lookup neither verifies nor enforces that the chunk we have retrieved
is the correct one. For this argument, I was not completely sure how
to put it into words first, so I have asked Claude about a good
definition regarding this point, to be told that direct pointers carry
no "identity", and I'm finding the term adapted here, because a value
acts as an identity to ensure that we have the chunk we expect, based
on the data on heap side.

My main assumption regarding this patch would be, mapping with
previous remarks I got, to use a reloption to decide which type of
external pointer to use and have a one-one mapping with what's stored
in heap rather than make the TOAST table definitions more complicated
than they should be. So: don't try to solve the rewrite problem now
and discard it, give the option for new tables to choose this method
(for the reasons listed in the last two paragraphs, I guess no anyway,
but that's what I would recommend if following up).

Saying all that, and after screening the patch set, there are two
things that I find attractive out of the bat.

Number 1, 0001. Making toast_fetch_datum() an inline function that
calls toast_fetch_datum_slice() sounds like an okay thing to do. Just
removing comments for the sake of moving code is never nice, or just
move the definition of toast_fetch_datum() to be closer to _slice().
Some could also claim about the code lacking symmetry with
toast_decompress_datum() and toast_decompress_datum_slice(), as well,
so we may also group these together..

Number 2, this thing, hidden in v2-0003 (for some reason, but the
split of the patch set is super weird to me, so I'm not quite sure to
follow entirely why you've done things this way for a couple of
parts):
+typedef struct ToastExternalMetadata
+{

It sounds to me that we could do that kind of thing *before* thinking
about adding new types of external pointers, because it simplifies the
data fetch in quite a few code paths? You could just rework that based
on HEAD, with only OID values around.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2026-09-07 03:21:27 Re: SUM(int2)/SUM(int4) do not detect overflow of the int8 accumulator
Previous Message Chao Li 2026-09-07 03:13:18 Re: Fix detection of truncated zstd-compressed backups