Horton

Re: [HACKERS] Custom compression methods

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>, 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>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-02-19T23:10:09Z
Lists: pgsql-hackers 
On Fri, Feb 19, 2021 at 4:21 PM Robert Haas <robertmhaas@gmail.com> wrote:
> What makes me a bit uncomfortable about that approach is that it
> presupposes that everything that uses expanded records has some other
> defense against those tuples getting written to disk without first
> expanding any external datums. And it isn't obvious that this is the
> case, or at least not to me. For example, PLpgsql's
> coerce_function_result_tuple()'s code for
> VARATT_IS_EXTERNAL_EXPANDED() has three cases. The first case passes
> the tuple through SPI_returntuple() which calls
> heap_copy_tuple_as_datum() which calls toast_flatten_tuple_to_datum()
> if required, but the second case calls EOH_flatten_into() and does NOT
> pass the result through SPI_returntuple(). And ER_flatten_info() has
> no defense against this case that I can see: sure, it skips the fast
> path if ER_FLAG_HAVE_EXTERNAL is set, but that doesn't actually do
> anything to resolve TOAST pointers. Maybe there's no bug there for
> some reason, but I don't know what that reason might be. We seem to
> have no test cases either in the main test suite or in the plpgsql
> test suite where ER_flatten_info gets called with
> ER_FLAG_HAVE_EXTERNAL is set, which seems a little unfortunate. If
> there is such a bug here it's independent of this patch, I suppose,
> but it would still be nice to understand what's going on here better
> than I do.

Andres just pointed out to me the error of my thinking here:
ER_flatten_into can *never* encounter a case with both
ER_FLAG_FVALUE_VALID and ER_FLAG_HAVE_EXTERNAL, because
ER_get_flat_size has to get called first, and will de-toast external
values as it goes. So there actually is justification for
coerce_function_result_tuple() to skip the call to SPI_returntuple().

Given that, one might wonder why the test in ER_flatten_into() even
cares about ER_FLAG_HAVE_EXTERNAL in the first place... I suppose it's
just a harmless oversight.

-- 
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.