Re: [HACKERS] Custom compression methods

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(at)gmail(dot)com> wrote:
> > On Sun, Jan 10, 2021 at 10:59 PM Justin Pryzby <pryzby(at)telsasoft(dot)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(at)telsasoft(dot)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 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).

--
Justin