ZSTD TOAST compression, and an extensible compression method encoding

On Mon, Sep 21, 2026 at 07:26:20PM -0700, Nikhil Kumar Veldanda wrote:

0001: Refactor TOAST compression header handling
------------------------------------------------

Preparatory, with no behavior change, so that 0002 only has to touch
one place for each thing it changes:

- toast_internals.h duplicated the layout of the compressed-in-line
header as toast_compress_header, with TOAST_COMPRESS_* macros to read
and write the tcinfo word, while varatt.h describes the same bytes as
varattrib_4b.va_compressed. The duplicate is removed and the header
is written through one inline function,
toast_compress_set_size_and_method().

+static varlena *
+toast_pointer_build(vartag_external tag, const void *fixed, Size fixedsize)

Hmm, why not. It is a pattern in two places. You raise it to 4, one
for each vartag.

+++ b/src/include/access/detoast.h
[...]
+#include "access/toast_compression.h"
[...]
+   ToastCompressionId compress_method; /* compression method ID */ 

So you need this new header dependency due to ToastCompressionId.
Okay.

- The compression method of an external value was fetched by callers
from the raw bits of va_extinfo. It is now decoded once, in
toast_external_info_get(), into a new compress_method field of
toast_external_data, and detoast_attr_slice(),
toast_get_compression_id() and amcheck read it from there.

- VARTAG_IS_ONDISK() is added next to VARTAG_IS_EXPANDED(), and the
TOAST pointer assembly at the end of toast_save_datum() moves into a
helper, toast_pointer_build().

How about splitting 0001 into a few more pieces, for clarity? It
looks like we could do the removal of toast_compress_header to the new
toast_compress_set_size_and_method() as a change of its own. The
introduction of toast_pointer_build() is a second one. I am not sure
that the new VARTAG_IS_ONDISK() buys us much, we have already too many
of these. Adding compress_method to toast_external_data and reducing
the footprint of VARATT_EXTINFO_GET_COMPRESS_METHOD() would be a third
one.

0002: Allow more than four TOAST compression methods
----------------------------------------------------

Seems like HEAD disagrees with this one in varatt.h and amcheck.

So a long-form pointer is 19 or 23 bytes instead of 18 or 22, and the
inline header is 9 bytes instead of 8, for methods other than pglz and
lz4 only. This leaves room for method IDs up to 255.

With 8-byte alignment in mind, this does not bother me in heap, be it
for oid-based varlenas or oid8-based varlenas.

The inline long form is described by a new struct:
typedef struct
{
uint32 va_header;
uint32 va_tcinfo; /* method bits =
VARLENA_COMPRESS_METHOD_LONG */
uint8 va_cmid; /* compression method ID */
char va_data[FLEXIBLE_ARRAY_MEMBER];
} varattrib_4b_long;

This is the fundamental part of the change for me. We just have one
bit left that gets redirected to this new "long" flavor, where we have
to pay a penalty of 1 byte to be able to store compression methods
other than pglz and lz4.

It is deliberately not a member of the varattrib_4b union. A member
with the extra byte pads to 12 bytes and would grow
sizeof(varattrib_4b) from 8, and a lot of code inspects varlena
headers of unknown or smaller size through pointers of that type;
Outside the union the struct's trailing padding is harmless, since
like the rest of varatt.h it is only ever used with offsetof() and
field access, never sizeof(). Two static assertions tie its va_tcinfo
and va_cmid offsets to va_compressed, so the "va_compressed plus one
byte" relationship is checked by the compiler rather than by a
comment.

I guess that's fine by me, on top of my mind. I am wondering if
others have different opinions and/or ideas about these structures?

0003: Add zstd as a TOAST compression method
--------------------------------------------

The backend-side core change goes at 200 lines or so with the two
first refactorings in place, where most of it consists of the full
compression and decompression and slice handling.

Not in this series: a compression level setting, and dictionary
support, which was the motivation for making the ID space large and
would be another method ID using the same long form.

Honestly, I don't care much about the level setting. If really
needed, we could bundle that as a GUC but i really doubt that people
are going to bother about tuning it. The default is usually good
enough, as support for compression of full pages in WAL has
demonstrated over the years.

An integration with dictionaries worry me a lot, because only somewhat
make sense when attached at tuple-level, and they evolve depending on
how zstd see fit. What worries me the most is the amount of work
between the on-disk data and the backend in terms of retention of past
dictionnaries.
--
Michael

Thanks Michael for feedback.

On Wed, Sep 23, 2026 at 11:12 PM Michael Paquier <michael@paquier.xyz> wrote:

+static varlena *
+toast_pointer_build(vartag_external tag, const void *fixed, Size fixedsize)

Hmm, why not. It is a pattern in two places. You raise it to 4, one
for each vartag.

Right, that is what made a helper worthwhile: with the long tags it
would otherwise be four copies of the same three lines.

How about splitting 0001 into a few more pieces, for clarity? It
looks like we could do the removal of toast_compress_header to the new
toast_compress_set_size_and_method() as a change of its own. The
introduction of toast_pointer_build() is a second one. I am not sure
that the new VARTAG_IS_ONDISK() buys us much, we have already too many
of these. Adding compress_method to toast_external_data and reducing
the footprint of VARATT_EXTINFO_GET_COMPRESS_METHOD() would be a third
one.

Done, in that order, in the attached v3:

0001 removes toast_compress_header and the TOAST_COMPRESS_* macros in
favor of the varatt.h accessors and
toast_compress_set_size_and_method().
0002 adds toast_pointer_build().
0003 adds compress_method to toast_external_data and makes the three
callers read it from there.
0004 and 0005 are the format change and zstd, as before.

VARTAG_IS_ONDISK() is gone. With two tags the comparison is fine as
HEAD spells it, so the three refactoring patches no longer touch
varatt.h at all.

0004 adds two VARTAG_IS_* helpers, down from four in v2:

- VARTAG_IS_ONDISK_OID8() is true for the two oid8 tags, plain and
long. toast_external_info_get() uses it to pick the oid8 struct, and
amcheck uses it to compare the tag with the chunk_id type of the TOAST
table.

- VARTAG_IS_ONDISK_LONG() is true for the two long tags. It tells
toast_external_info_get() to read the method byte,
toast_pointer_build() to write it, and amcheck that the value must be
compressed.

Each one compares the tag with its two enum values by name. The
low-bit trick is gone, so the numbering of the long tags next to the
plain ones is only a mnemonic now. VARATT_IS_EXTERNAL_ONDISK() accepts
all four tags, and amcheck's "unexpected TOAST tag" check calls it
instead of keeping its own list.

0002: Allow more than four TOAST compression methods
----------------------------------------------------

Seems like HEAD disagrees with this one in varatt.h and amcheck.

Sorry about that. v2 was generated from a tree that predated
be00f041a33, which touched the two comment lines that the patch edits.
v3 applies on HEAD as of 4545cee303c. While there I fixed a doubled
word that my reflow of the amcheck comment had introduced.

The inline long form is described by a new struct:
typedef struct
{
uint32 va_header;
uint32 va_tcinfo; /* method bits =
VARLENA_COMPRESS_METHOD_LONG */
uint8 va_cmid; /* compression method ID */
char va_data[FLEXIBLE_ARRAY_MEMBER];
} varattrib_4b_long;

This is the fundamental part of the change for me. We just have one
bit left that gets redirected to this new "long" flavor, where we have
to pay a penalty of 1 byte to be able to store compression methods
other than pglz and lz4.

Yes. That is the whole of the format change; the rest of 0004 is
plumbing for it.

Honestly, I don't care much about the level setting. If really
needed, we could bundle that as a GUC but i really doubt that people
are going to bother about tuning it. The default is usually good
enough, as support for compression of full pages in WAL has
demonstrated over the years.

Agreed, it stays out. Nothing in the format depends on it, so it can
be added later as a GUC or an attribute option.

An integration with dictionaries worry me a lot, because only somewhat
make sense when attached at tuple-level, and they evolve depending on
how zstd see fit. What worries me the most is the amount of work
between the on-disk data and the backend in terms of retention of past
dictionnaries.

I understand that, and this series does not try to answer it. How
dictionaries would be stored, versioned and retired is a separate
design, and I'd rather not touch it until plain zstd is in.

Michael

--
Nikhil Veldanda

On Thu, Sep 24, 2026 at 04:00:29PM -0700, Nikhil Kumar Veldanda wrote:

0001 removes toast_compress_header and the TOAST_COMPRESS_* macros in
favor of the varatt.h accessors and
toast_compress_set_size_and_method().

In this one, I got mixed feelings about the addition of the new inline
routine in toast_internals.h while we have already a set of two getter
routines in varatt.h that interact with va_tcinfo, so I have moved an
equivalent to varatt.h. An extra benefit of this move is that we
don't need to include toast_compression.h in toast_internals.h
anymore. Applied the result. Cool cleanup, thanks for that.

0002 adds toast_pointer_build().

+static varlena *
+toast_pointer_build(vartag_external tag, const void *fixed, Size fixedsize)
+{
+    varlena    *result;
+
+    Assert(tag == VARTAG_ONDISK_OID || tag == VARTAG_ONDISK_OID8);
+    Assert(VARTAG_SIZE(tag) == fixedsize);
+
+    result = (varlena *) palloc(VARHDRSZ_EXTERNAL + VARTAG_SIZE(tag));
+    SET_VARTAG_EXTERNAL(result, tag);
+    memcpy(VARDATA_EXTERNAL(result), fixed, fixedsize);
+
+    return result;
+}

In this one, is there a need for fixedsize at all? We can guess the
size of the target based on the vartag and VARTAG_SIZE(), without
needing an assertion. If this brings any kind of extra protection, I
may have missed it..

Rename *fixed to *ptr? Should this use a VARATT_IS_EXTERNAL_ONDISK()
in the first assertion instead of declaring the two vartags?

0003 adds compress_method to toast_external_data and makes the three
callers read it from there.

 		/* Compressed attributes should have a valid compression method */
-		cmid = VARATT_EXTINFO_GET_COMPRESS_METHOD(toast_ext_data.extinfo);
+		cmid = toast_ext_data.compress_method;

It looks like we could remove VARATT_EXTINFO_GET_COMPRESS_METHOD().
That's kind of tempting..
--
Michael

On Fri, Sep 25, 2026 at 1:04 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Sep 24, 2026 at 04:00:29PM -0700, Nikhil Kumar Veldanda wrote:

0001 removes toast_compress_header and the TOAST_COMPRESS_* macros in
favor of the varatt.h accessors and
toast_compress_set_size_and_method().

In this one, I got mixed feelings about the addition of the new inline
routine in toast_internals.h while we have already a set of two getter
routines in varatt.h that interact with va_tcinfo, so I have moved an
equivalent to varatt.h. An extra benefit of this move is that we
don't need to include toast_compression.h in toast_internals.h
anymore. Applied the result. Cool cleanup, thanks for that.

Thanks. That is a better home for it, and it sets the pattern for the
long form: the format patch now adds
VARDATA_COMPRESSED_SET_TCINFO_LONG() next to
VARDATA_COMPRESSED_SET_TCINFO() in varatt.h, and
toast_compress_datum() picks one or the other based on
toast_compression_id_needs_cmid_byte(). The series does not touch
toast_internals.h at all anymore. One small change to the routine you
committed: its assertion on the method becomes "cmethod <
VARLENA_COMPRESS_METHOD_LONG", since from that patch on the value 3 is
a flag for the long form, not a method.

0002 adds toast_pointer_build().

+static varlena *
+toast_pointer_build(vartag_external tag, const void *fixed, Size fixedsize)
+{
+    varlena    *result;
+
+    Assert(tag == VARTAG_ONDISK_OID || tag == VARTAG_ONDISK_OID8);
+    Assert(VARTAG_SIZE(tag) == fixedsize);
+
+    result = (varlena *) palloc(VARHDRSZ_EXTERNAL + VARTAG_SIZE(tag));
+    SET_VARTAG_EXTERNAL(result, tag);
+    memcpy(VARDATA_EXTERNAL(result), fixed, fixedsize);
+
+    return result;
+}

In this one, is there a need for fixedsize at all? We can guess the
size of the target based on the vartag and VARTAG_SIZE(), without
needing an assertion. If this brings any kind of extra protection, I
may have missed it..

Only a cross-check that the struct the caller filled is the one the
tag announces; without it, a mismatch would read four bytes past the
caller's local. With both callers sitting right next to the struct
they fill, that is not worth a parameter. Removed, and the size now
comes from VARTAG_SIZE(). In the format patch the position of the
method byte is derived from it as well, so the helper's inputs are
down to the tag, the struct and the method.

Rename *fixed to *ptr? Should this use a VARATT_IS_EXTERNAL_ONDISK()
in the first assertion instead of declaring the two vartags?

Both done. VARATT_IS_EXTERNAL_ONDISK() wants a datum rather than a
tag, so the assertion moves after SET_VARTAG_EXTERNAL() and checks the
result. A nice side effect is that the format patch does not need to
touch that assertion when it adds the long tags.

0003 adds compress_method to toast_external_data and makes the three
callers read it from there.

/* Compressed attributes should have a valid compression method */
-               cmid = VARATT_EXTINFO_GET_COMPRESS_METHOD(toast_ext_data.extinfo);
+               cmid = toast_ext_data.compress_method;

It looks like we could remove VARATT_EXTINFO_GET_COMPRESS_METHOD().
That's kind of tempting..

Done in that patch. Once the callers read toast_external_data, the
only user left was toast_external_info_get() itself, which now shifts
the bits directly, and keeping the function around would only invite
code to bypass the struct. The format patch was removing it anyway,
so this just moves it earlier; the comment on
VARATT_EXTINFO_GET_EXTSIZE() points at compress_method instead.

v4 attached, rebased on e27f3b2cad7 and renumbered now that the first
patch is in:

0001 adds toast_pointer_build().
0002 adds compress_method to toast_external_data and removes
VARATT_EXTINFO_GET_COMPRESS_METHOD().
0003 is the format change.
0004 is zstd.

--
Michael

--
Nikhil Veldanda

Hi Nikhil,

Thanks for the updated patch set! Looking at patch 4/4, I noticed a minor
issue in zstd_compress_datum() regarding the check for incompressible data:

/* data is incompressible so just free the memory and return NULL */
if (len > (size_t) valsize)
{
pfree(tmp);
return NULL;
}
Should this condition be >= instead of >? if (len >= (size_t) valsize)
Reasons:
If len == valsize, the compressed payload alone is already the same size as
the uncompressed data. Once VARHDRSZ_COMPRESSED_LONG (9 bytes) is added,
the compressed datum is strictly larger than the original. There is no
compression benefit here.
If len == valsize, zstd_compress_datum() currently proceeds to call
SET_VARSIZE_COMPRESSED() and returns tmp, only for the caller
toast_compress_datum() to immediately reject it via if (VARSIZE(tmp) <
valsize - 2) and pfree(tmp). Freeing it early and returning NULL saves
redundant operations.
This is also consistent with lz4_compress_datum() in the same file:

/*
* If the compressed size is greater than or equal to the raw data size,
* then data is incompressible so just free the memory and return NULL.
*/
if (len >= valsize)
{
pfree(tmp);
return NULL;
}

On Fri, Sep 25, 2026 at 5:35 AM wenhui qiu <qiuwenhuifx@gmail.com> wrote:

Hi Nikhil,

Thanks for the updated patch set! Looking at patch 4/4, I noticed a minor issue in zstd_compress_datum() regarding the check for incompressible data:

/* data is incompressible so just free the memory and return NULL */
if (len > (size_t) valsize)
{
pfree(tmp);
return NULL;
}
Should this condition be >= instead of >? if (len >= (size_t) valsize)
Reasons:
If len == valsize, the compressed payload alone is already the same size as the uncompressed data. Once VARHDRSZ_COMPRESSED_LONG (9 bytes) is added, the compressed datum is strictly larger than the original. There is no compression benefit here.
If len == valsize, zstd_compress_datum() currently proceeds to call SET_VARSIZE_COMPRESSED() and returns tmp, only for the caller toast_compress_datum() to immediately reject it via if (VARSIZE(tmp) < valsize - 2) and pfree(tmp). Freeing it early and returning NULL saves redundant operations.

The check in the compressors is deliberately loose. The real decision
is made once for all methods in toast_compress_datum(), which knows
about the header and the alignment margin and rejects anything that
does not save more than 2 bytes with the header included. So for zstd
every result from len == valsize - 11 upwards is thrown away there.
Changing > to >= in the compressor catches one of those twelve
lengths; the other eleven still take the round trip. To actually
avoid it, each compressor would need a copy of the caller's threshold,
which is what keeping the policy in one place is meant to avoid.

The saving would also be one store and one comparison in a case that
does not happen in practice: zstd's frame overhead means
incompressible input comes out strictly longer than valsize.

This is also consistent with lz4_compress_datum() in the same file:

/*
* If the compressed size is greater than or equal to the raw data size,
* then data is incompressible so just free the memory and return NULL.
*/
if (len >= valsize)
{
pfree(tmp);
return NULL;
}

lz4_compress_datum() in master has "if (len > valsize)", unchanged
since bbe0a81db69 added it. The zstd check is a copy of it, so the
two are consistent as they stand.

--
Nikhil Veldanda

Hi, Nikhil

On Fri, 25 Sep 2026 at 02:21, Nikhil Kumar Veldanda <veldanda.nikhilkumar17@gmail.com> wrote:

On Fri, Sep 25, 2026 at 1:04 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Sep 24, 2026 at 04:00:29PM -0700, Nikhil Kumar Veldanda wrote:

0001 removes toast_compress_header and the TOAST_COMPRESS_* macros in
favor of the varatt.h accessors and
toast_compress_set_size_and_method().

In this one, I got mixed feelings about the addition of the new inline
routine in toast_internals.h while we have already a set of two getter
routines in varatt.h that interact with va_tcinfo, so I have moved an
equivalent to varatt.h. An extra benefit of this move is that we
don't need to include toast_compression.h in toast_internals.h
anymore. Applied the result. Cool cleanup, thanks for that.

Thanks. That is a better home for it, and it sets the pattern for the
long form: the format patch now adds
VARDATA_COMPRESSED_SET_TCINFO_LONG() next to
VARDATA_COMPRESSED_SET_TCINFO() in varatt.h, and
toast_compress_datum() picks one or the other based on
toast_compression_id_needs_cmid_byte(). The series does not touch
toast_internals.h at all anymore. One small change to the routine you
committed: its assertion on the method becomes "cmethod <
VARLENA_COMPRESS_METHOD_LONG", since from that patch on the value 3 is
a flag for the long form, not a method.

0002 adds toast_pointer_build().

+static varlena *
+toast_pointer_build(vartag_external tag, const void *fixed, Size fixedsize)
+{
+    varlena    *result;
+
+    Assert(tag == VARTAG_ONDISK_OID || tag == VARTAG_ONDISK_OID8);
+    Assert(VARTAG_SIZE(tag) == fixedsize);
+
+    result = (varlena *) palloc(VARHDRSZ_EXTERNAL + VARTAG_SIZE(tag));
+    SET_VARTAG_EXTERNAL(result, tag);
+    memcpy(VARDATA_EXTERNAL(result), fixed, fixedsize);
+
+    return result;
+}

In this one, is there a need for fixedsize at all? We can guess the
size of the target based on the vartag and VARTAG_SIZE(), without
needing an assertion. If this brings any kind of extra protection, I
may have missed it..

Only a cross-check that the struct the caller filled is the one the
tag announces; without it, a mismatch would read four bytes past the
caller's local. With both callers sitting right next to the struct
they fill, that is not worth a parameter. Removed, and the size now
comes from VARTAG_SIZE(). In the format patch the position of the
method byte is derived from it as well, so the helper's inputs are
down to the tag, the struct and the method.

Rename *fixed to *ptr? Should this use a VARATT_IS_EXTERNAL_ONDISK()
in the first assertion instead of declaring the two vartags?

Both done. VARATT_IS_EXTERNAL_ONDISK() wants a datum rather than a
tag, so the assertion moves after SET_VARTAG_EXTERNAL() and checks the
result. A nice side effect is that the format patch does not need to
touch that assertion when it adds the long tags.

0003 adds compress_method to toast_external_data and makes the three
callers read it from there.

/* Compressed attributes should have a valid compression method */
-               cmid = VARATT_EXTINFO_GET_COMPRESS_METHOD(toast_ext_data.extinfo);
+               cmid = toast_ext_data.compress_method;

It looks like we could remove VARATT_EXTINFO_GET_COMPRESS_METHOD().
That's kind of tempting..

Done in that patch. Once the callers read toast_external_data, the
only user left was toast_external_info_get() itself, which now shifts
the bits directly, and keeping the function around would only invite
code to bypass the struct. The format patch was removing it anyway,
so this just moves it earlier; the comment on
VARATT_EXTINFO_GET_EXTSIZE() points at compress_method instead.

v4 attached, rebased on e27f3b2cad7 and renumbered now that the first
patch is in:

0001 adds toast_pointer_build().
0002 adds compress_method to toast_external_data and removes
VARATT_EXTINFO_GET_COMPRESS_METHOD().
0003 is the format change.
0004 is zstd.

In the v4-0001 commit message, it says no behavior change. However, it seems
an Assert introduced after setting vartag.

Since the access/detoast.h include the access/toast_compression.h, should we remove
the access/toast_compression.h in files that already include access/toash.h?

$ grep -rl '#include[[:space:]]*[<"]access/detoast\.h[>"]' . \
| xargs grep -l '#include[[:space:]]*[<"]access/toast_compression\.h[>"]'
./src/backend/access/brin/brin_tuple.c
./src/backend/access/common/toast_compression.c
./src/backend/access/common/detoast.c
./src/backend/access/common/toast_internals.c
./src/backend/utils/adt/varlena.c
./contrib/amcheck/verify_heapam.c

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Hi Japin,

On Fri, Sep 25, 2026 at 8:25 PM Japin Li <japinli@hotmail.com> wrote:

In the v4-0001 commit message, it says no behavior change. However, it seems
an Assert introduced after setting vartag.

Fair. The assertion cannot fire (both callers pass a constant tag)
and is compiled out without --enable-cassert, but I'll mention it in
the commit message next time.

Since the access/detoast.h include the access/toast_compression.h, should we remove
the access/toast_compression.h in files that already include access/toash.h?

I'd keep them. All six files use toast_compression.h symbols
directly, and the convention is to include what you use rather than
rely on a transitive include (src/tools/pginclude/README). detoast.h
includes it for its own field, and e27f3b2cad7 added these includes to
exactly these files for the same reason.

--
Nikhil Veldanda

Hi Nikhil,

v4 attached, rebased on e27f3b2cad7 and renumbered now that the first
patch is in:

I ran some checks and numbers on v4, on master 3c5d9d914fa. Scripts
and the full tables are attached.

On-disk format. The cover letter says pglz and lz4 values keep their
representation bit for bit. Writing the same data with master and
with v4, the raw bytes of the inline compressed datums (pageinspect)
and of the TOAST chunks hash the same, for pglz and lz4. pg_upgrade
from master to v4 keeps every value, and verify_heapam(check_toast)
finds nothing. It is also clean on v4 for a column holding pglz, lz4
and zstd values at once, and after VACUUM FULL, UPDATE and DELETE on
a zstd table.

No slowdown for the existing methods. v4 against master with lz4,
paired within each of 6 rounds: the median ratio is between 0.97 and
1.06 for every point, and no point is slower in all rounds. The two
rounds with pglz look the same.

zstd itself, on -O2 builds, hot cache, 200 MB per case, sizes as a
percentage of the raw data (pglz / lz4 / zstd):

- SGML docs in 32 kB values: 35.7% / 41.0% / 26.9%
- JSON documents in 1 MB values: 12.1% / 18.0% / 4.6%
- base64 text in 32 kB values: 102.9% / 102.9% / 77.0%

The last one is worth noting: pglz and lz4 get nothing out of base64
or hex text, and zstd saves about a quarter.

The cost is on reads of small values. For 50,000 JSON values of
4 kB, reading them all takes 140 ms with zstd against 53 ms with lz4,
and a 100-byte prefix costs the same as the whole value (140 ms,
against 23 ms with lz4).

One thing in 0004 that is easy to improve:
zstd_decompress_datum_slice() creates and frees a ZSTD_DCtx for every
value. The attached diff keeps one per backend, reset on each use,
and uses it in both decompression paths. zstd, 4 rounds, ratio of
the diff to v4:

- 32 kB values, slice and prefix: 0.66 to 0.77, faster in every round
- 32 kB values, full read: 0.87 to 0.89 for docs and base64
- 1 MB base64 values, slice: 0.70
- 4 kB values: 0.94 to 0.97

make check passes with it. At 4 kB the prefix still costs as much as
the whole value, so what is left there is per frame, not the context.

Regards,
Manu

On Fri, Sep 25, 2026 at 10:21:26PM -0700, Nikhil Kumar Veldanda wrote:

On Fri, Sep 25, 2026 at 8:25 PM Japin Li <japinli@hotmail.com> wrote:

In the v4-0001 commit message, it says no behavior change. However, it seems
an Assert introduced after setting vartag.

Fair. The assertion cannot fire (both callers pass a constant tag)
and is compiled out without --enable-cassert, but I'll mention it in
the commit message next time.

I'm not really sure that this is worth mentioning at all. You can
qualify that as a "change" because it adds one line, yes, but it's
just here to make sure that nobody does something stupid with the new
routine introduced by v4-0001, and the code is still the same as the
original version, just de-duplicated.

Applied this one.

Since the access/detoast.h include the access/toast_compression.h, should we remove
the access/toast_compression.h in files that already include access/toash.h?

I'd keep them. All six files use toast_compression.h symbols
directly, and the convention is to include what you use rather than
rely on a transitive include (src/tools/pginclude/README). detoast.h
includes it for its own field, and e27f3b2cad7 added these includes to
exactly these files for the same reason.

Hmm. I'd say that this depends. Cleanups of dependencies in .h files
are usually kind of nice, because it shows a cleaner chain of
dependencies at header level. If you'd like to suggest a patch,
please feel free. I am not sure how much this is worth it without
looking at some code shaping your idea.
--
Michael

HI Nikhil

Thanks for the clarification! You are completely right about the caller
threshold
policy in toast_compress_datum() and the zstd frame overhead, and my
apologies
for misquoting lz4_compress_datum(). Keeping the compressors loose and
centralizing the storage policy makes total sense.
While studying the patch series further, I noticed a few technical points
regarding error handling, extensibility, and edge-case slicing that might
be
worth looking into:
1. Potential slice-fetch length mismatch in detoast.c (Patch 3/4)
In detoast.c, toast_fetch_datum_slice() accounts for the compressed header
overhead with:
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && slicelength > 0)
slicelength = slicelength + sizeof(int32);
For plain compressed datums (pglz/lz4), the header in chunk 0 is va_tcinfo
(4 bytes). But for the long-form encoding introduced in patch 3,
toast_save_datum()
stores va_tcinfo (4 bytes) PLUS va_cmid (1 byte), totaling 5 bytes
(VARHDRSZ_COMPRESSED_LONG - VARHDRSZ).
Currently, detoast_attr_slice() fetches the entire external datum for lz4
and zstd
so the length is clamped at attrsize, but if toast_fetch_datum_slice() is
used
to fetch a true slice of a long-form external datum, hardcoding
sizeof(int32)
would result in fetching 1 byte short of the payload.
Should this instead be computed based on
VARTAG_IS_ONDISK_LONG(toast_pointer)?
Size hdrsize = VARTAG_IS_ONDISK_LONG(toast_ext_data.tag) ?
(VARHDRSZ_COMPRESSED_LONG - VARHDRSZ) :
(VARHDRSZ_COMPRESSED - VARHDRSZ);
slicelength += hdrsize;
2. Assertion failure during amcheck on corrupted on-disk datums (Patch 3)
In detoast.h, toast_external_info_get() has:
if (VARTAG_IS_ONDISK_LONG(toast_ext_data->tag))
{
uint8 cmid;
Assert(toast_ext_data->compress_method ==
VARLENA_COMPRESS_METHOD_LONG);
memcpy(&cmid, ptr + fixedsize, sizeof(cmid));
toast_ext_data->compress_method = (ToastCompressionId) cmid;
}
Because amcheck calls toast_external_info_get() to verify disk tuples, if
an on-disk
datum is corrupted such that the tag is LONG but the va_extinfo bits were
corrupted
to something else, an assert-enabled build will crash with AssertionFailed
instead of
letting amcheck catch and report the corruption.
In contrast, VARDATA_COMPRESSED_GET_COMPRESS_METHOD() in varatt.h uses a
non-asserting
"if (method == VARLENA_COMPRESS_METHOD_LONG)". Perhaps
toast_external_info_get()
should do the same and set compress_method to TOAST_INVALID_COMPRESSION_ID
on mismatch?
3. Hardcoded method enum in toast_save_datum() (Patch 3 & 4)
In toast_internals.c:
Assert(cmid == TOAST_PGLZ_COMPRESSION_ID ||
cmid == TOAST_LZ4_COMPRESSION_ID ||
cmid == TOAST_ZSTD_COMPRESSION_ID);
if (toast_compression_id_needs_cmid_byte(cmid))
Since the goal of patch 3 was to allow extensible compression methods
without
hardcoding IDs across toast internals, should this assert simply be:
Assert(cmid != TOAST_INVALID_COMPRESSION_ID);
which matches the asserts in toast_compress_datum() and
toast_pointer_build()?
4. Defensive underflow guard for VARSIZE in zstd_decompress_datum() (Patch
4)
In zstd_decompress_datum():
rawsize = ZSTD_decompress(VARDATA(result),
VARDATA_COMPRESSED_GET_EXTSIZE(value),
(const char *) value +
VARHDRSZ_COMPRESSED_LONG,
VARSIZE(value) - VARHDRSZ_COMPRESSED_LONG);
Unlike LZ4_decompress_safe() where the compressed size argument is a
signed int
(which immediately returns error if negative), ZSTD_decompress() takes
size_t
(unsigned). If a corrupted on-disk datum has VARSIZE(value) <
VARHDRSZ_COMPRESSED_LONG,
this underflows to a massive unsigned value, which could cause
ZSTD_decompress()
to read out of bounds. Adding a check for VARSIZE(value) <
VARHDRSZ_COMPRESSED_LONG
before calling ZSTD_decompress() would be safer.
5. Decompressed size verification in zstd_decompress_datum() (Patch 4)
ZSTD_decompress() only returns an error if dstCapacity is too small; if
the frame
decompresses to fewer bytes than dstCapacity (the recorded extsize), it
returns the
smaller size without error.
To protect against corrupted or truncated streams, should we also verify
that
the decompressed size matches the recorded external size?
if (ZSTD_isError(rawsize) || rawsize != (size_t)
VARDATA_COMPRESSED_GET_EXTSIZE(value))
ereport(ERROR, ...);
Thanks again for driving this work!

Best regards

On Sat, Sep 26, 2026 at 12:25 PM Manu <manuelreyesbravo@gmail.com> wrote:

Hi Nikhil,

v4 attached, rebased on e27f3b2cad7 and renumbered now that the first
patch is in:

I ran some checks and numbers on v4, on master 3c5d9d914fa. Scripts
and the full tables are attached.

On-disk format. The cover letter says pglz and lz4 values keep their
representation bit for bit. Writing the same data with master and
with v4, the raw bytes of the inline compressed datums (pageinspect)
and of the TOAST chunks hash the same, for pglz and lz4. pg_upgrade
from master to v4 keeps every value, and verify_heapam(check_toast)
finds nothing. It is also clean on v4 for a column holding pglz, lz4
and zstd values at once, and after VACUUM FULL, UPDATE and DELETE on
a zstd table.

Thanks a lot for checking both, and for the scripts. Those are the
two things the format patch must not break.

The cost is on reads of small values. For 50,000 JSON values of
4 kB, reading them all takes 140 ms with zstd against 53 ms with lz4,
and a 100-byte prefix costs the same as the whole value (140 ms,
against 23 ms with lz4).

The prefix part comes from the format: a zstd block holds up to 128 kB
and can only be decoded whole, so for any smaller value a prefix is a
full decode. v7 has a comment saying so above the slice loop.

One thing in 0004 that is easy to improve:
zstd_decompress_datum_slice() creates and frees a ZSTD_DCtx for every
value. The attached diff keeps one per backend, reset on each use,
and uses it in both decompression paths. zstd, 4 rounds, ratio of
the diff to v4:

I agree reusing the contexts is worth doing; zstd itself recommends
it, and ZSTD_compress() allocates one per call too, so the write side
would gain as well. For this series I'd rather stay with what the
tree already does: the WAL code uses the one-shot ZSTD_compress() and
ZSTD_decompress() calls, and 0003 matches that. A context kept for
the backend's lifetime also brings its own questions (how much memory
it may keep, and when to release it), which I'd rather settle
separately than attach to the format change.

So I'll propose it as a follow-up once this series is in, starting
from your diff and your numbers, if that works for you.

Regards,
Manu

--
Nikhil Veldanda

On Mon, Sep 28, 2026 at 3:01 AM wenhui qiu <qiuwenhuifx@gmail.com> wrote:

HI Nikhil

Thanks for the clarification! You are completely right about the caller threshold
policy in toast_compress_datum() and the zstd frame overhead, and my apologies
for misquoting lz4_compress_datum(). Keeping the compressors loose and
centralizing the storage policy makes total sense.

No problem, thanks for looking again.

1. Potential slice-fetch length mismatch in detoast.c (Patch 3/4)
In detoast.c, toast_fetch_datum_slice() accounts for the compressed header
overhead with:
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && slicelength > 0)
slicelength = slicelength + sizeof(int32);
For plain compressed datums (pglz/lz4), the header in chunk 0 is va_tcinfo
(4 bytes). But for the long-form encoding introduced in patch 3, toast_save_datum()
stores va_tcinfo (4 bytes) PLUS va_cmid (1 byte), totaling 5 bytes
(VARHDRSZ_COMPRESSED_LONG - VARHDRSZ).
Currently, detoast_attr_slice() fetches the entire external datum for lz4 and zstd
so the length is clamped at attrsize, but if toast_fetch_datum_slice() is used
to fetch a true slice of a long-form external datum, hardcoding sizeof(int32)
would result in fetching 1 byte short of the payload.
Should this instead be computed based on VARTAG_IS_ONDISK_LONG(toast_pointer)?

Yes. It can't happen today because only pglz fetches a true prefix
and pglz never uses the long form, but the code shouldn't depend on
that. Fixed in the format patch: the header size now follows the tag.

2. Assertion failure during amcheck on corrupted on-disk datums (Patch 3)
In detoast.h, toast_external_info_get() has:
if (VARTAG_IS_ONDISK_LONG(toast_ext_data->tag))
{
uint8 cmid;
Assert(toast_ext_data->compress_method == VARLENA_COMPRESS_METHOD_LONG);
memcpy(&cmid, ptr + fixedsize, sizeof(cmid));
toast_ext_data->compress_method = (ToastCompressionId) cmid;
}
Because amcheck calls toast_external_info_get() to verify disk tuples, if an on-disk
datum is corrupted such that the tag is LONG but the va_extinfo bits were corrupted
to something else, an assert-enabled build will crash with AssertionFailed instead of
letting amcheck catch and report the corruption.
In contrast, VARDATA_COMPRESSED_GET_COMPRESS_METHOD() in varatt.h uses a non-asserting
"if (method == VARLENA_COMPRESS_METHOD_LONG)". Perhaps toast_external_info_get()
should do the same and set compress_method to TOAST_INVALID_COMPRESSION_ID on mismatch?

Yes. I reproduced it by clearing the method bits of a long pointer on
disk, the kind of damage src/bin/pg_amcheck/t/004_verify_heapam.pl
injects to check that amcheck reports corruption without crashing. An
assert-enabled build then hits TRAP: failed Assert(...) in
verify_heapam(); a production build doesn't crash, but reads the
method byte anyway and reports nothing. Fixed as you suggest: a
mismatch now decodes as TOAST_INVALID_COMPRESSION_ID, and amcheck
reports "toast value 16399 has invalid compression method id 3", the
same way it already reports a plain pointer whose bits hold 0b11.

3. Hardcoded method enum in toast_save_datum() (Patch 3 & 4)
In toast_internals.c:
Assert(cmid == TOAST_PGLZ_COMPRESSION_ID ||
cmid == TOAST_LZ4_COMPRESSION_ID ||
cmid == TOAST_ZSTD_COMPRESSION_ID);
if (toast_compression_id_needs_cmid_byte(cmid))
Since the goal of patch 3 was to allow extensible compression methods without
hardcoding IDs across toast internals, should this assert simply be:
Assert(cmid != TOAST_INVALID_COMPRESSION_ID);
which matches the asserts in toast_compress_datum() and toast_pointer_build()?

I'd keep the list. Master already lists pglz and lz4 there. Unlike
in toast_compress_datum(), where cmid comes from the switch just
above, here it is read from the datum, so the list also catches values
that are not methods at all, which "!= INVALID" would let through. A
new method missing from it fails its first test, so it can't go stale.
The format patch is about deciding the pointer form in one place, not
about never naming methods; the dispatch switches and amcheck name
them too.

4. Defensive underflow guard for VARSIZE in zstd_decompress_datum() (Patch 4)
In zstd_decompress_datum():
rawsize = ZSTD_decompress(VARDATA(result),
VARDATA_COMPRESSED_GET_EXTSIZE(value),
(const char *) value + VARHDRSZ_COMPRESSED_LONG,
VARSIZE(value) - VARHDRSZ_COMPRESSED_LONG);
Unlike LZ4_decompress_safe() where the compressed size argument is a signed int
(which immediately returns error if negative), ZSTD_decompress() takes size_t
(unsigned). If a corrupted on-disk datum has VARSIZE(value) < VARHDRSZ_COMPRESSED_LONG,
this underflows to a massive unsigned value, which could cause ZSTD_decompress()
to read out of bounds. Adding a check for VARSIZE(value) < VARHDRSZ_COMPRESSED_LONG
before calling ZSTD_decompress() would be safer.

Agreed, added to both zstd decompressors. pglz and lz4 get away with
it because they take a signed size.

5. Decompressed size verification in zstd_decompress_datum() (Patch 4)
ZSTD_decompress() only returns an error if dstCapacity is too small; if the frame
decompresses to fewer bytes than dstCapacity (the recorded extsize), it returns the
smaller size without error.

Not quite. A zstd datum records its uncompressed size twice: in our
header (va_tcinfo) and in the zstd frame header, which ZSTD_compress()
always fills in. zstd checks its own copy: a truncated frame, or one
that decodes to a different length than it records, is an error.
Checked against libzstd 1.5.7, a frame cut by one byte fails with "Src
size is incorrect", and one whose recorded size I raised fails with
"Data corruption detected".

To protect against corrupted or truncated streams, should we also verify that
the decompressed size matches the recorded external size?

Yes, because zstd never sees our copy of the size: we only pass it as
dstCapacity, and a destination larger than needed is not an error. So
if our header claims more than the frame holds, zstd returns the real
data without complaint, and the mismatch goes unnoticed.

That has visible effects. In a manual test (not part of the patch), I
raised the size in the header of an inline 3450-byte zstd datum from
3450 to 3460 in the heap file, leaving the frame alone. Without the
check, the value reads back as the correct 3450 bytes, yet "v = <the
original text>" returns false: texteq() compares lengths first, via
toast_raw_datum_size(), which for a compressed datum takes the size
from our header without decompressing. With the check, the read fails
with "compressed zstd data is corrupt" instead. pglz already does the
same, through check_complete; lz4 doesn't. The slice path can't check
this, since it stops before the end by design.

v7 attached

--
Nikhil Veldanda

On Mon, Sep 28, 2026 at 10:39:42PM -0700, Nikhil Kumar Veldanda wrote:

v7 attached

I was just looking at v7-0001 that wants to add the ToastCompressionId
to toast_external_data, and this feels half-baked due to the
inconsistency this brings with extsize and VARATT_EXTINFO_GET_EXTSIZE.

Couldn't we do better here by normalizing more data from the existing
fields? Another could be the is_compressed state which is guessed
from a comparison between the raw size and the compressed size,
perhaps?

Regarding v7-0002 and v7-0003, I am doubting the wisdom of tackling
the last-compression-bit issue for this release. The OID8 code has
already changed a lot of code, and maybe we should be conservative in
terms of the amount of the changes we do in this area for a single
release. By that, I mean to catch up on the refactoring pieces on
this thread once some dust has settled on HEAD and tackle this issue
around the time v21 opens up (Aka I'm preparing myself for these
dozens of agents to complain about the shape of the code already
committed, we'll see how it goes).
--
Michael

Hi everyone

I have been working on a more generic approach to compression and
similar encoding/decoding tasks

I have put my current thinking on this in
https://wiki.postgresql.org/wiki/Pg_encoding_context

This is meant to provide generic support functionality for things like
compression dictionaries for zstd and lz4, Avro schemas, huffman
tables and vector quanttization base data to name a few

I appreciate any feedback on the approach

---
Hannu

Hi Michael, Nikhil,

I was just looking at v7-0001 that wants to add the ToastCompressionId
to toast_external_data, and this feels half-baked due to the
inconsistency this brings with extsize and VARATT_EXTINFO_GET_EXTSIZE.

Couldn't we do better here by normalizing more data from the existing
fields? Another could be the is_compressed state which is guessed
from a comparison between the raw size and the compressed size,
perhaps?

Looking closely at v7-0001 with this in mind, I completely agree. Once we
introducing toast_external_data to decouple callers from the low-level on-disk
representation, keeping extsize and is_compressed as raw macro calculations
for callers to repeat leaves the abstraction half-baked.
In fact, there is an even stronger argument for normalizing is_compressed
here: currently, when an external datum is uncompressed, the high 2 bits of
va_extinfo are zero (since payload size < 1GB). If a caller inspects
toast_ext_data.compress_method without manually checking
VARATT_EXTINFO_IS_COMPRESSED() first, they end up seeing 0
(TOAST_PGLZ_COMPRESSION_ID) instead of an invalid ID.
If we normalize both extsize and is_compressed inside
toast_external_info_get(),
we can explicitly enforce:
```
typedef struct toast_external_data
{
vartag_external tag;
int32 rawsize;
int32 extsize; /* unpacked external payload size */
bool is_compressed; /* true if extsize < rawsize - VARHDRSZ */
ToastCompressionId compress_method; /* valid only if is_compressed */
Oid8 valueid;
Oid toastrelid;
uint32 extinfo; /* preserved raw bits */
} toast_external_data;
And in toast_external_info_get():

toast_ext_data->extsize = toast_ext_data->extinfo & VARLENA_EXTSIZE_MASK;
toast_ext_data->is_compressed =
(toast_ext_data->extsize < toast_ext_data->rawsize - VARHDRSZ);
if (toast_ext_data->is_compressed)
{
/* resolve compress_method, handling long form if tag demands */
...
}
else
toast_ext_data->compress_method = TOAST_INVALID_COMPRESSION_ID;
```

I checked the codebase: doing this cleans up repetitive calls to
VARATT_EXTINFO_GET_EXTSIZE() and VARATT_EXTINFO_IS_COMPRESSED() across
at least 11 sites in detoast.c, toast_internals.c, amcheck, and
reorderbuffer.c. It makes the whole subsystem much cleaner.

Regarding v7-0002 and v7-0003, I am doubting the wisdom of tackling
the last-compression-bit issue for this release. The OID8 code has
already changed a lot of code, and maybe we should be conservative in
terms of the amount of the changes we do in this area for a single
release. By that, I mean to catch up on the refactoring pieces on this
thread once some dust has settled on HEAD and tackle this issue around
the time v21 opens up

From a release management standpoint, taking a conservative stance
here makes total sense. The OID8 expansion was a major physical
storage milestone, and avoided simultaneous churn on on-disk TOAST
pointer formats right after it minimized risk to HEAD.

At the same time, I want to thank Nikhil for the great work on v7:
having re-reviewed the patch series against latest HEAD (commit
45277ca0d1c) and verified it locally, all edge cases previously
discussed (the slice fetch offset calculation, amcheck assertion
guards on corrupt pointers, unsigned underflow checks, and header size
verification) are cleanly resolved, and the entire test suite passes
without issues.

Given that, I think splitting the timeline as Michael suggested is the
best path forward:

Enhance 0001 into a standalone, fully-normalized toast_external_data
refactoring and commit it in the current cycle. It carries zero
on-disk format changes and strictly improves the codebase.
Hold 0002 and 0003 until HEAD settles and v21 opens up. Since
0002/0003 are already technically solid, having a fully-normalized
toast_external_data in place now will make landing the long-form
format and zstd in v21 remarkably straightforward.
Nikhil, what do you think about updating 0001 along these lines for a v8?

Best regards

Hi Nikhil,

I've been reviewing v7 and it looks solid to me. Full disclosure: Nikhil
and I work together at AWS and used to be on the same team, which is where
this patch set started, so I'm a fan and also biased.

So, I applied v7 to master as of 54635e6892f and everything worked as
expected.

Two points on v7:

0001: For an external value that isn't compressed, the new compress_method
field comes back as 0, which happens to mean pglz. Anyone who reads the
field without first checking VARATT_EXTINFO_IS_COMPRESSED() will think
the value is pglz-compressed. I'd have toast_external_info_get() fill in
the size and the compressed flag as well, and set compress_method to
TOAST_INVALID_COMPRESSION_ID when there's no compression. That's
essentially what Michael and wenhui suggested, and it would make 0001
useful on its own this cycle.

0003: You held off on Manu's context reuse because the WAL code used the
one-shot zstd calls. That's no longer true: Michael's 857340b0263 and
ce3387b74b7 made WAL keep its zstd contexts for the life of the backend.
Given Manu's numbers (slices 23-34% faster, full reads of 32kB values
11-13% faster), I'd fold something similar to his change into 0003.

I know, I know... but why not talk about them?

Dictionaries
------------

You and Michael have both put dictionaries out of scope until plain zstd is
in, and that is the right order for commits. Michael's concern upthread
still stands:

What worries me the most is the amount of work between the on-disk
data and the backend in terms of retention of past dictionnaries.

The first thread stopped on the same problem. Robert's objections to v11's
pg_depend tracking [1]/messages/by-id/CA+Tgmob1Ux7R2_0mbZmTbsVmUda04X1AdHGjpu6FkZfdbztugQ@mail.gmail.com, Andres's point about undroppable dictionaries
accumulating [2]/messages/by-id/wjrd7ubqm4sq6t4ddv2ae7xaf26vcf4z6i7nftag3e6zexqmyc@yjilk5fsqcfh, and the per-insert cost of v13's flatten_datum [3]/messages/by-id/CAFAfj_Fee7zQ7YNnJObLMqVF=MpFwVHszzPJ-ZWkiiade04SAw@mail.gmail.com were
deferred rather than answered. 0002 spends the last free value of the method
bits on the grounds that dictionary support would be another method ID in the
long form. It seems worth showing before 0002 lands that a dictionary method
with a workable lifecycle fits that form.

The attached v20261015-0004 is a patch on top of v7 that does that, following
Robert's suggestion in [4]/messages/by-id/CA+TgmoY8uCKFvVYDQsLbfhY3JYrduXE7WHi75hmfTWxjWioaaw@mail.gmail.com: dictionary-compressed datums exist only in the
column that owns the dictionary, and are decompressed whenever they would be
stored anywhere else. 0001-0003 of the attached series are your v7, unchanged
apart from the rebase onto aa7b9cf132b.

Ownership. A new catalog, pg_toast_dictionary (oid, tdrelid, tdattnum,
tdcurrent, tddict), whose rows belong to one column as pg_statistic rows do.
DROP TABLE removes them, next to RemoveStatistics(). DROP COLUMN does not: the
dropped column's values remain in the heap, and whole-row references and
REPLICA IDENTITY FULL still read them, until the next rewrite retires the
dictionaries. There are no pg_depend entries.

Containment. heap_fill_tuple() sets HEAP_HASEXTERNAL on a tuple holding a
zstd_dict datum, as it does for a TOAST pointer. Every path that already
flattens such tuples (toast_flatten_tuple() and its variants, expanded records,
the catcache, heap_update()'s need_toast) therefore reaches the datum, and
detoast_external_attr() decompresses it. The few places that test
VARATT_IS_EXTERNAL() on a single datum (index_form_tuple, brin_form_tuple,
tstoreReceiver, PL/pgSQL non-atomic assignment) also test
VARATT_IS_COMPRESSED_DICT().

On store, toast_tuple_init() keeps a zstd_dict datum compressed only if the
target column owns the dictionary and the dictionary is current; otherwise it
decompresses it and the target column's method applies. Values compressed with
pglz, lz4 or plain zstd are copied as before, so the CTAS slowdown seen with
v13's flatten_datum applies only when the source column uses a dictionary.

Retirement. pg_train_toast_dictionary(rel, col, dict_size) trains from the
column's values and makes the new dictionary current; the previous one stays
readable. The next rewrite (VACUUM FULL, CLUSTER, REPACK, or a rewriting ALTER
TABLE) recompresses values that use a non-current dictionary, and
finish_heap_swap() then deletes it. Dictionaries of a column switched to
another method, or dropped, are retired the same way. Rewrites already hold
AccessExclusiveLock. Training takes ShareUpdateExclusiveLock, as ANALYZE does.

Upgrade. pg_upgrade keeps the heap files, which embed dictionary OIDs, so
binary-upgrade dumps restore each dictionary with its OID and physical column
number, including those of switched or dropped columns that have not been
rewritten yet.

On disk, a zstd_dict datum is the v7 long header plus the dictionary OID:

va_header | va_tcinfo (method bits 11) | va_cmid = 4 | dictid | frame

so 0002 needs no change for it.

I've not yet toyed with the following, maybe you have ideas?

- Training is an explicit call over the first 10000 rows. Training from
ANALYZE's sample is the natural next step.
- toast_dictionary_current() does an index probe per compressed value,
and the CDict/DDict cache holds one entry each way. Both can move to
the relcache entry if profiling shows a need.
- Prefix slices of dictionary values decode the whole value, which costs
nothing extra below zstd's 128kB block size.
- No compression level setting.

Hannu's encoding-context proposal [5]https://wiki.postgresql.org/wiki/Pg_encoding_context is broader. I'd expect the ownership and
containment rules here to apply to it as well.

If this matches what you had in mind for dictionaries, I'd suggest carrying it
as a later patch in your series, so the long-form discussion has a concrete
dictionary user to point at. I can rebase it on v8.

best.

-greg

[1]: /messages/by-id/CA+Tgmob1Ux7R2_0mbZmTbsVmUda04X1AdHGjpu6FkZfdbztugQ@mail.gmail.com
[2]: /messages/by-id/wjrd7ubqm4sq6t4ddv2ae7xaf26vcf4z6i7nftag3e6zexqmyc@yjilk5fsqcfh
[3]: /messages/by-id/CAFAfj_Fee7zQ7YNnJObLMqVF=MpFwVHszzPJ-ZWkiiade04SAw@mail.gmail.com
[4]: /messages/by-id/CA+TgmoY8uCKFvVYDQsLbfhY3JYrduXE7WHi75hmfTWxjWioaaw@mail.gmail.com
[5]: https://wiki.postgresql.org/wiki/Pg_encoding_context