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-04T22:20:49Z
Lists: pgsql-hackers 
On Thu, Feb 4, 2021 at 11:39 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> Yeah, actually, I thought I would avoid calling slot_getallattrs if
> none of the attributes got decompress.  I agree if we call this before
> we can avoid calling slot_getattr but slot_getattr
> is only called for the attribute which has attlen -1. I agree that if
> we call slot_getattr for attnum n then it will deform all the
> attributes before that.  But then slot_getallattrs only need to deform
> the remaining attributes not all.  But maybe we can call the
> slot_getallattrs as soon as we see the first attribute with attlen -1
> and then avoid calling subsequent slot_getattr, maybe that is better
> than compared to what I have because we will avoid calling
> slot_getattr for many attributes, especially when there are many
> verlena.

I think that if we need to deform at all, we need to deform all
attributes, right? So there's no point in considering e.g.
slot_getsomeattrs(). But just slot_getallattrs() as soon as we know we
need to do it might be worthwhile. Could even have two loops: one that
just figures out whether we need to deform; if not, return. Then
slot_getallattrs(). Then another loop to do the work.

>  I think the supported procedure for this sort of
> > thing is to have a second slot, set tts_values, tts_isnull etc. and
> > then materialize the slot. After materializing the new slot, it's
> > independent of the old slot, which can then be cleared. See for
> > example tts_virtual_materialize().
>
> Okay, so if we take a new slot then we need to set this slot reference
> in the ScanState also otherwise that might point to the old slot.  I
> haven't yet analyzed where all we might be keeping the reference to
> that old slot.  Or I am missing something.

My guess is you want to leave the ScanState alone so that we keep
fetching into the same slot as before and have an extra slot on the
side someplace.

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