MIME-Version: 1.0
References: 
 <CAOj6k6dy2CRVA6Lsb5N59zE-7KNVKt=oYwWyg8ULK8zOOY8e7A@mail.gmail.com>
 <CAOj6k6dkseVvZzmEAWvBd6twZsCU0DbN+qeM7CoDuMM3r9doiw@mail.gmail.com>
 <CAOj6k6eSsQ_PTUTxW0upn6wp7tzbvQwqkQco-=+majwX8p6JJg@mail.gmail.com>
 <4c70b0f6-5aba-464c-b145-464a620c1222@eisentraut.org>
 <aUOgmypuun0rc2xO@paquier.xyz>
 <CAOj6k6fxs0Bwjr34W4aURzFPta0DTGLN-1ic-U+-M_EqJ+Wd8A@mail.gmail.com>
 <aUSDs7SaEcOa65gD@paquier.xyz>
 <CABV9wwOdeFHHNnw+rUsVnFek-oqwsJnJX9KFG6AT5Yqz5RA_vg@mail.gmail.com>
 <aUyEWfBXYBLKGkKQ@paquier.xyz>
In-Reply-To: <aUyEWfBXYBLKGkKQ@paquier.xyz>
From: Dharin Shah <dharinshah95@gmail.com>
Date: Thu, 25 Dec 2025 01:54:46 +0100
Message-ID: 
 <CAOj6k6dG0M-G0gs_Htray5y_pfvacXSjKYw0uJaaYJvOeqk7xw@mail.gmail.com>
Subject: Re: Fwd: [PATCH] Add zstd compression for TOAST using extended header
 format
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Treat <rob@xzilla.net>, Peter Eisentraut <peter@eisentraut.org>,
	pgsql-hackers@lists.postgresql.org
Content-Type: multipart/alternative; boundary="000000000000d48a3f0646bc3998"
Archived-At: 
 <https://www.postgresql.org/message-id/CAOj6k6dG0M-G0gs_Htray5y_pfvacXSjKYw0uJaaYJvOeqk7xw%40mail.gmail.com>
Precedence: bulk

--000000000000d48a3f0646bc3998
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Thanks Michael & Robert,

Agreed =E2=80=94 I don=E2=80=99t think it=E2=80=99s 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=E2=80=99d end=
 up with
multiple =E2=80=9Csources of truth=E2=80=9D 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=E2=80=99t 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 =E2=80=9Ctranslation structure=E2=80=
=9D approach you
mentioned in the other thread? If you can point me to it, I=E2=80=99ll alig=
n 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=E2=80=AFAM Michael Paquier <michael@paquier.xy=
z> 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
>

--000000000000d48a3f0646bc3998
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Thanks Michael &amp; Robert,<br><br>Agree=
d =E2=80=94 I don=E2=80=99t think it=E2=80=99s realistic or practical to ta=
lk about deprecating or replacing pglz (or lz4) given on-disk compatibility=
 requirements.<br><br>&gt; Note that I am not on board with simply reusing =
varatt_external for<br>&gt; zstd-compressed entries, neither do I think tha=
t this is the best move<br>&gt;=C2=A0ever.=C2=A0 It makes the core patch si=
mpler, but it makes things like<br>&gt;=C2=A0ToastCompressionId more compli=
cated to think about.=C2=A0 If anything, I&#39;d<br>&gt;=C2=A0consider a re=
name of varatt_external as the best way to go with an<br>&gt;=C2=A0intermed=
iate &quot;translation&quot; structure only used in memory as I am<br>&gt;=
=C2=A0proposing on the other thread (something that others seem meh enough<=
br>&gt;=C2=A0about but I am not seeing alternate proposals floating around,=
<br>&gt;=C2=A0either).=C2=A0 This would make things like detoast_external_a=
ttr() less<br>&gt;=C2=A0confusing, I think, as the latest patch posted on t=
his thread is<br>&gt;=C2=A0actually proving with its shortcut for toast_fet=
ch_datum as one<br>&gt;=C2=A0example of something I&#39;d rather not do..<b=
r><br>On the design: I understand &amp; share the same concerns that we=E2=
=80=99d end up with multiple =E2=80=9Csources of truth=E2=80=9D for externa=
l compression method identification (pglz/lz4 via va_extinfo bits, zstd via=
 vartag), and that this pushes method-specific shortcuts into detoast paths=
.<br><br>Would you be OK if I split this into two steps?<br><br>1.First, a =
refactor-only patch introducing a small decoded/in-memory representation of=
 an external TOAST pointer, so detoast/toast code paths don=E2=80=99t have =
to reason directly about tcinfo vs vartag vs va_extinfo. This would be a cl=
eanup with no on-disk format change and no behavioral change for existing m=
ethods. Is this the same =E2=80=9Ctranslation structure=E2=80=9D approach y=
ou mentioned in the other thread? If you can point me to it, I=E2=80=99ll a=
lign with that proposal.<br><br>2. Then, a follow-up patch adding zstd usin=
g VARTAG_ONDISK_ZSTD, implemented on top of that abstraction to keep zstd h=
andling centralized and minimize special-casing in detoast.<br>If that dire=
ction matches what you had in mind, I can first post the proposed translati=
on structure/API for feedback before respinning the zstd patch.<br><br>Than=
ks,<br>Dharin<br><div><div><br></div></div></div><br><div class=3D"gmail_qu=
ote gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Thu, De=
c 25, 2025 at 1:25=E2=80=AFAM Michael Paquier &lt;<a href=3D"mailto:michael=
@paquier.xyz">michael@paquier.xyz</a>&gt; wrote:<br></div><blockquote class=
=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rg=
b(204,204,204);padding-left:1ex">On Wed, Dec 24, 2025 at 11:50:48AM -0500, =
Robert Treat wrote:<br>
&gt; Agreed that I can&#39;t see pglz being removed any time soon, if ever.=
<br>
&gt; Thinking through what a conversion process would look like seems<br>
&gt; unwieldy at best, so I think we definitely need it for backwards<br>
&gt; compatibility, plus I think it is useful to have a self-contained<br>
&gt; option. I&#39;d almost suggest we should look at replacing lz4, but I<=
br>
&gt; don&#39;t think that is significantly easier, it just has a smaller, m=
ore<br>
&gt; invested, blast radius.<br>
<br>
Backward-compatibility requirements make a replacement of LZ4<br>
basically impossible to me, for the same reasons as pglz.=C2=A0 We could<br=
>
not replace the bit used in the va_extinfo to track if LZ4 compression<br>
is used, either.=C2=A0 One thing that I do wonder is if it would make<br>
things simpler in the long-run if we introduced a new separated vartag<br>
for LZ4-compressed external TOAST pointers as well.=C2=A0 At least we&#39;d=
<br>
have a leaner design: it means that we have to keep the<br>
varatt_external available on read, but we could update to the new<br>
format when writing entries.=C2=A0 Or perhaps that&#39;s not worth the<br>
complication based on the last sentence you are writing..=C2=A0 <br>
<br>
&gt; That said, I do suspect ztsd could quickly<br>
&gt; become a popular recommendation and/or default among users /<br>
&gt; consultants / service providers.<br>
<br>
..=C2=A0 Because I strongly suspect that this is going to be true, and that=
<br>
zstd would just be a better replacement over lz4.=C2=A0 That&#39;s a trend =
that<br>
I see is already going on for wal_compression.<br>
<br>
Note that I am not on board with simply reusing varatt_external for<br>
zstd-compressed entries, neither do I think that this is the best move<br>
ever.=C2=A0 It makes the core patch simpler, but it makes things like<br>
ToastCompressionId more complicated to think about.=C2=A0 If anything, I=
9;d<br>
consider a rename of varatt_external as the best way to go with an<br>
intermediate &quot;translation&quot; structure only used in memory as I am<=
br>
proposing on the other thread (something that others seem meh enough<br>
about but I am not seeing alternate proposals floating around,<br>
either).=C2=A0 This would make things like detoast_external_attr() less<br>
confusing, I think, as the latest patch posted on this thread is<br>
actually proving with its shortcut for toast_fetch_datum as one<br>
example of something I&#39;d rather not do..<br>
--<br>
Michael<br>
</blockquote></div></div>

--000000000000d48a3f0646bc3998--