Horton

Re: [HACKERS] Custom compression methods

Dilip Kumar <dilipbalaut@gmail.com>

View thread View on postgr.es
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Robert Haas <robertmhaas@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-19T16:21:57Z
Lists: pgsql-hackers 
On Thu, Feb 11, 2021 at 3:26 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> In CompareCompressionMethodAndDecompress, I think this is still
> playing a bit fast and loose with the rules around slots. I think we
> can do better. Suppose that at the point where we discover that we
> need to decompress at least one attribute, we create the new slot
> right then, and also memcpy tts_values and tts_isnull. Then, for that
> attribute and any future attributes that need decompression, we reset
> tts_values in the *new* slot, leaving the old one untouched. Then,
> after finishing all the attributes, the if (decompressed_any) block,
> you just have a lot less stuff to do. The advantage of this is that
> you haven't tainted the old slot; it's still got whatever contents it
> had before, and is in a clean state, which seems better to me.

Fixed

>
> It's unclear to me whether this function actually needs to
> ExecMaterializeSlot(newslot). It definitely does need to
> ExecStoreVirtualTuple(newslot) and I think it's a very good idea, if
> not absolutely mandatory, for it not to modify anything about the old
> slot. But what's the argument that the new slot needs to be
> materialized at this point? It may be needed, if the old slot would've
> had to be materialized at this point. But it's something to think
> about.

I think if the original slot was materialized then materialing the new
slot make more sense to me so done that way.

>
> The CREATE TABLE documentation says that COMPRESSION is a kind of
> column constraint, but that's wrong. For example, you can't write
> CREATE TABLE a (b int4 CONSTRAINT thunk COMPRESSION lz4), for example,
> contrary to what the syntax summary implies. When you fix this so that
> the documentation matches the grammar change, you may also need to
> move the longer description further up in create_table.sgml so the
> order matches.

Fixed

> The use of VARHDRSZ_COMPRESS in toast_get_compression_oid() appears to
> be incorrect. VARHDRSZ_COMPRESS is offsetof(varattrib_4b,
> va_compressed.va_data). But what gets externalized in the case of a
> compressed datum is just VARDATA(dval), which excludes the length
> word, unlike VARHDRSZ_COMPRESS, which does not. This has no
> consequences since we're only going to fetch 1 chunk either way, but I
> think we should make it correct.

Fixed

> TOAST_COMPRESS_SET_SIZE_AND_METHOD() could Assert something about cm_method.

While replying to the comments, I realised that I have missed it.  I
will fix it in the next version.

> Small delta patch with a few other suggested changes attached.

Merged

-- 
Regards,
Dilip Kumar
EnterpriseDB: 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.