Re: Fwd: [PATCH] Add zstd compression for TOAST using extended header format

Thanks Michael & Robert,

Agreed — I don’t think it’s realistic or practical to talk about
deprecating or replacing pglz (or lz4) given on-disk compatibility
requirements.

> Note that I am not on board with simply reusing varatt_external for
> zstd-compressed entries, neither do I think that this is the best move
> ever. It makes the core patch simpler, but it makes things like
> ToastCompressionId more complicated to think about. If anything, I'd
> consider a rename of varatt_external as the best way to go with an
> intermediate "translation" structure only used in memory as I am
> proposing on the other thread (something that others seem meh enough
> about but I am not seeing alternate proposals floating around,
> either). This would make things like detoast_external_attr() less
> confusing, I think, as the latest patch posted on this thread is
> actually proving with its shortcut for toast_fetch_datum as one
> example of something I'd rather not do..

On the design: I understand & share the same concerns that we’d end up with
multiple “sources of truth” for external compression method identification
(pglz/lz4 via va_extinfo bits, zstd via vartag), and that this pushes
method-specific shortcuts into detoast paths.

Would you be OK if I split this into two steps?

1.First, a refactor-only patch introducing a small decoded/in-memory
representation of an external TOAST pointer, so detoast/toast code paths
don’t have to reason directly about tcinfo vs vartag vs va_extinfo. This
would be a cleanup with no on-disk format change and no behavioral change
for existing methods. Is this the same “translation structure” approach you
mentioned in the other thread? If you can point me to it, I’ll align with
that proposal.

2. Then, a follow-up patch adding zstd using VARTAG_ONDISK_ZSTD,
implemented on top of that abstraction to keep zstd handling centralized
and minimize special-casing in detoast.
If that direction matches what you had in mind, I can first post the
proposed translation structure/API for feedback before respinning the zstd
patch.

Thanks,
Dharin

On Thu, Dec 25, 2025 at 1:25 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Wed, Dec 24, 2025 at 11:50:48AM -0500, Robert Treat wrote:
> > Agreed that I can't see pglz being removed any time soon, if ever.
> > Thinking through what a conversion process would look like seems
> > unwieldy at best, so I think we definitely need it for backwards
> > compatibility, plus I think it is useful to have a self-contained
> > option. I'd almost suggest we should look at replacing lz4, but I
> > don't think that is significantly easier, it just has a smaller, more
> > invested, blast radius.
>
> Backward-compatibility requirements make a replacement of LZ4
> basically impossible to me, for the same reasons as pglz. We could
> not replace the bit used in the va_extinfo to track if LZ4 compression
> is used, either. One thing that I do wonder is if it would make
> things simpler in the long-run if we introduced a new separated vartag
> for LZ4-compressed external TOAST pointers as well. At least we'd
> have a leaner design: it means that we have to keep the
> varatt_external available on read, but we could update to the new
> format when writing entries. Or perhaps that's not worth the
> complication based on the last sentence you are writing..
>
> > That said, I do suspect ztsd could quickly
> > become a popular recommendation and/or default among users /
> > consultants / service providers.
>
> .. Because I strongly suspect that this is going to be true, and that
> zstd would just be a better replacement over lz4. That's a trend that
> I see is already going on for wal_compression.
>
> Note that I am not on board with simply reusing varatt_external for
> zstd-compressed entries, neither do I think that this is the best move
> ever. It makes the core patch simpler, but it makes things like
> ToastCompressionId more complicated to think about. If anything, I'd
> consider a rename of varatt_external as the best way to go with an
> intermediate "translation" structure only used in memory as I am
> proposing on the other thread (something that others seem meh enough
> about but I am not seeing alternate proposals floating around,
> either). This would make things like detoast_external_attr() less
> confusing, I think, as the latest patch posted on this thread is
> actually proving with its shortcut for toast_fetch_datum as one
> example of something I'd rather not do..
> --
> Michael
>