Re: [BUG] pg_upgrade test fails from older versions.

From: "Anton A(dot) Melnikov" <aamelnikov(at)inbox(dot)ru>
To: Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [BUG] pg_upgrade test fails from older versions.
Date: 2022-12-22 06:59:18
Message-ID: f9d65c82-ebef-4ff8-1055-b1b704a19995@inbox.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello!

Divided patch into two parts: first part refers to the modification of
the old dump while the second one relates to dump filtering.

1) v2-0001-Remove-aclitem-from-old-dump.patch

On 19.12.2022 06:10, Michael Paquier wrote:
> This is forgetting about materialized views, which is something that
> pg_upgrade would also complain about when checking for relations with
> aclitems. As far as I can see, the only place in the main regression
> test suite where we have an aclitem attribute is tab_core_types for
> HEAD and the stable branches, so it would be enough to do this
> change. Anyway, wouldn't it be better to use the same conditions as
> the WITH OIDS queries a few lines above, at least for consistency?
>
> Note that check_for_data_types_usage() checks for tables, matviews and
> indexes.

Found that 'ALTER ... ALTER COLUMN SET DATA TYPE text'
is not applicable to materialized views and indexes as well as DROP COLUMN.
So couldn't make anything better than drop its in the old dump if they
contain at least one column of 'aclitem' type.

i've tested this script with:
CREATE TABLE acltable AS SELECT 1 AS int, 'postgres=a/postgres'::aclitem AS aclitem;
CREATE MATERIALIZED VIEW aclmview AS SELECT 'postgres=a/postgres'::aclitem AS aclitem;
CREATE INDEX aclindex on acltable (int) INCLUDE (aclitem);
performed in the regression database before creating the old dump.

The only thing i haven't been able to find a case when an an 'acltype' column would
be preserved in the index when this type was replaced in the parent table.
So passing relkind = 'i' is probably redundant.
If it is possible to find such a case, it would be very interesting.

Also made the replacement logic for 'acltype' in the tables more closer
to above the script that removes OIDs columns. In this script found likewise that
ALTER TABLE ... SET WITHOUT OIDS is not applicable to materialized views
and ALTER MATERIALIZED VIEW doesn't support WITHOUT OIDS clause.
Besides i couldn't find any legal way to create materialized view with oids in versions 11 or lower.
Command 'CREATE MATERIALIZED VIEW' doesn't support WITH OIDS or (OIDS) clause,
as well as ALTER MATERIALIZED VIEW as mentioned above.
Even with GUC default_with_oids = true":
CREATE TABLE oidtable AS SELECT 1 AS int;
CREATE MATERIALIZED VIEW oidmv AS SELECT * FROM oidtable;
give:
postgres=# SELECT oid::regclass::text FROM pg_class WHERE relname !~ '^pg_' AND relhasoids;
oid
----------
oidtable
(1 row)
So suggest to exclude the check of materialized views from this DO block.
Would be grateful for remarks if i didn't consider some cases.

2) v2-0002-Additional-dumps-filtering.patch

On 19.12.2022 06:16, Michael Paquier wrote:
>
> While thinking about that, an extra idea popped in my mind as it may
> be interesting to be able to filter out some of the diffs in some
> contexts. So what about adding in 002_pg_upgrade.pl a small-ish hook
> in the shape of a new environment variable pointing to a file adds
> some custom filtering rules?

Yes. Made a hook that allows to proceed an external text file with additional
filtering rules and example of such file. Please take a look on it.

With the best wishes,

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
v2-0001-Remove-aclitem-from-old-dump.patch text/x-patch 2.4 KB
v2-0002-Additional-dumps-filtering.patch text/x-patch 2.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-12-22 07:05:46 Re: Force streaming every change in logical decoding
Previous Message Tom Lane 2022-12-22 06:10:54 Re: Error-safe user functions