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-11T05:30:14Z
Lists: pgsql-hackers 
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 think this patch needs to be specifically concerned with pg_upgrade, so I
> suggest to not drop your tables and MVs, to allow the pg_upgrade test to check
> them.  That exposes this issue:

Thanks for the suggestion I will try this.

> pg_dump: error: Error message from server: ERROR:  cache lookup failed for access method 36447
> pg_dump: error: The command was: COPY public.cmdata (f1) TO stdout;
> pg_dumpall: error: pg_dump failed on database "regression", exiting
> waiting for server to shut down.... done
> server stopped
> pg_dumpall of post-upgrade database cluster failed
>
> 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.


> In my brief attempt to inspect it, I got this crash:
>
> $ tmp_install/usr/local/pgsql/bin/postgres -D src/bin/pg_upgrade/tmp_check/data &
> regression=# SELECT pg_column_compression(f1) FROM cmdata a;
> server closed the connection unexpectedly
>
> Thread 1 "postgres" received signal SIGSEGV, Segmentation fault.
> __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
> 120     ../sysdeps/x86_64/multiarch/../strlen.S: No such file or directory.
> (gdb) bt
> #0  __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:120
> #1  0x000055c6049fde62 in cstring_to_text (s=0x0) at varlena.c:193
> #2  pg_column_compression () at varlena.c:5335
>
> (gdb) up
> #2  pg_column_compression () at varlena.c:5335
> 5335            PG_RETURN_TEXT_P(cstring_to_text(get_am_name(
> (gdb) l
> 5333            varvalue = (struct varlena *) DatumGetPointer(value);
> 5334
> 5335            PG_RETURN_TEXT_P(cstring_to_text(get_am_name(
> 5336                                                                    toast_get_compression_oid(varvalue))));
>
> I guess a missing AM here is a "shouldn't happen" case, but I'd prefer it to be
> caught with an elog() (maybe in get_am_name()) or at least an Assert.

Yeah, this makes sense.

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