Horton

Re: [HACKERS] Custom compression methods (mac+lz4.h)

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Andres Freund <andres@anarazel.de>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>, David Steele <david@pgmasters.net>, Ildus Kurbangaliev <i.kurbangaliev@gmail.com>, Dmitry Dolgov <9erthalion6@gmail.com>, Andrey Borodin <x4mmm@yandex-team.ru>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-22T15:29:10Z
Lists: pgsql-hackers 
On Mon, Mar 22, 2021 at 10:41 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Okay, the fix makes sense.  In fact, IMHO, in general also this fix
> > looks like an optimization, I mean when slicelength >=
> > VARRAWSIZE_4B_C(value), then why do we need to allocate extra memory
> > even in the case of pglz.  So shall we put this check directly in
> > toast_decompress_datum_slice instead of handling it at the lz4 level?
>
> Yeah, I thought about that too, but do we want to assume that
> VARRAWSIZE_4B_C is the correct way to get the decompressed size
> for all compression methods?

I think it's OK to assume this. If and when we add a third compression
method, it seems certain to just grab one of the two remaining bit
patterns. Now, things get a bit more complicated if and when we want
to add a fourth method, because at that point you've got to ask
yourself how comfortable you feel about stealing the last bit pattern
for your feature. But, if the solution to that problem were to decide
that whenever that last bit pattern is used, we will add an extra byte
(or word) after va_tcinfo indicating the real compression method, then
using VARRAWSIZE_4B_C here would still be correct. To imagine this
decision being wrong, you have to posit a world in which one of the
two remaining bit patterns for the high 2 bits cause the low 30 bits
to be interpreted as something other than the size, which I guess is
not totally impossible, but my first reaction is to think that such a
design would be (1) hard to make work and (2) unnecessarily painful.

> (If so, I think it would be better style to have a less opaque macro
> name for the purpose.)

Complaining about the name of one particular TOAST-related macro name
seems a bit like complaining about the greenhouse gasses emitted by
one particular car. They're pretty uniformly terrible. Does anyone
really know when to use VARATT_IS_1B_E or VARATT_IS_4B_U or any of
that cruft? Like, who decided that "is this varatt 1B E?" would be a
perfectly reasonable way of asking "is this varlena is TOAST
pointer?". While I'm complaining, it's hard to say enough bad things
about the fact that we have 12 consecutive completely obscure macro
definitions for which the only comments are (a) that they are
endian-dependent - which isn't even true for all of them - and (b)
that they are "considered internal." Apparently, they're SO internal
that they don't even need to be understandable to other developers.

Anyway, this particular macro name was chosen, it seems, for symmetry
with VARDATA_4B_C, but if you want to change it to something else, I'm
OK with that, too.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Commits

  1. docs: Update TOAST storage docs for configurable compression.

  2. Further tweaking of pg_dump's handling of default_toast_compression.

  3. Fix interaction of TOAST compression with expression indexes.

  4. Tidy up more loose ends related to configurable TOAST compression.

  5. Short-circuit slice requests that are for more than the object's size.

  6. Mostly-cosmetic adjustments of TOAST-related macros.

  7. Remove useless configure probe for <lz4/lz4.h>.

  8. Error on invalid TOAST compression in CREATE or ALTER TABLE.

  9. docs: Fix omissions related to configurable TOAST compression.

  10. More code cleanup for configurable TOAST compression.

  11. Bring configure support for LZ4 up to snuff.

  12. Make compression.sql regression test independent of default.

  13. Use valid compression method in brin_form_tuple

  14. Fix up pg_dump's handling of per-attribute compression options.

  15. Allow configurable LZ4 TOAST compression.

  16. Fix inconsistencies in the code

  17. Mostly-cosmetic improvements in memory chunk header alignment coding.

  18. Allow numeric to use a more compact, 2-byte header in many cases.