Horton

Re: [HACKERS] Custom compression methods

Dilip Kumar <dilipbalaut@gmail.com>

View thread View on postgr.es
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Robert Haas <robertmhaas@gmail.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-01-13T08:44:01Z
Lists: pgsql-hackers

Attachments

On Mon, Jan 11, 2021 at 3:40 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Jan 11, 2021 at 12:21 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > On Mon, Jan 11, 2021 at 12:11:54PM +0530, Dilip Kumar wrote:
> > > On Mon, Jan 11, 2021 at 11:00 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > On Sun, Jan 10, 2021 at 10:59 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > > >
> > > > > On Mon, Jan 04, 2021 at 04:57:16PM +0530, Dilip Kumar wrote:
> > > > > > On Mon, Jan 4, 2021 at 6:52 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > > > > > And fails pg_upgrade check, apparently losing track of the compression (?)
> > > > > > >
> > > > > > >  CREATE TABLE public.cmdata2 (
> > > > > > > -    f1 text COMPRESSION lz4
> > > > > > > +    f1 text
> > > > > > >  );
> > > > > >
> > > > > > I did not get this?  pg_upgrade check is passing for me.
> > > > >
> > > > > I realized that this was failing in your v16 patch sent Dec 25.
> > > > > It's passing on current patches because they do "DROP TABLE cmdata2", but
> > > > > that's only masking the error.
> > >
> > > I tested specifically pg_upgrade by removing all the DROP table and MV
> > > and it is passing.  I don't see the reason why should it fail.  I mean
> > > after the upgrade why COMPRESSION lz4 is missing?
> >
> > How did you test it ?
> >
> > I'm not completely clear how this is intended to work... has it been tested
> > before ?  According to the comments, in binary upgrade mode, there's an ALTER
> > which is supposed to SET COMPRESSION, but that's evidently not happening.
>
> I am able to reproduce this issue, If I run pg_dump with
> binary_upgrade mode then I can see the issue (./pg_dump
> --binary-upgrade   -d Postgres).  Yes you are right that for fixing
> this there should be an ALTER..SET COMPRESSION method.
>
> > > > > I found that's the AM's OID in the old clsuter:
> > > > > regression=# SELECT * FROM pg_am WHERE oid=36447;
> > > > >   oid  | amname |  amhandler  | amtype
> > > > > -------+--------+-------------+--------
> > > > >  36447 | pglz2  | pglzhandler | c
> > > > >
> > > > > But in the new cluster, the OID has changed.  Since that's written into table
> > > > > data, I think you have to ensure that the compression OIDs are preserved on
> > > > > upgrade:
> > > > >
> > > > >  16755 | pglz2  | pglzhandler          | c
> > > >
> > > > Yeah, basically we are storing am oid in the compressed data so Oid
> > > > must be preserved.  I will look into this and fix it.
> > >
> > > On further analysis, if we are dumping and restoring then we will
> > > compress the data back while inserting it so why would we need to old
> > > OID.  I mean in the new cluster we are inserting data again so it will
> > > be compressed again and now it will store the new OID.  Am I missing
> > > something here?
> >
> > I'm referring to pg_upgrade which uses pg_dump, but does *not* re-insert data,
> > but rather recreates catalogs only and then links to the old tables (either
> > with copy, link, or clone).  Test with make -C src/bin/pg_upgrade (which is
> > included in make check-world).
>
> Got this as well.
>
> I will fix these two issues and post the updated patch by tomorrow.
>
> Thanks for your findings.

I have fixed this issue in the v18 version, please test and let me
know your thoughts.  There is one more issue pending from an upgrade
perspective in v18-0003, basically, for the preserved method we need
to restore the dependency as well.  I will work on this part and
shared the next version soon.

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