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

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