Re: ZSTD TOAST compression, and an extensible compression method encoding

Thanks Michael for feedback.

On Wed, Sep 23, 2026 at 11:12 PM Michael Paquier <michael(at)paquier(dot)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