Re: [BUG] pg_upgrade test fails from older versions. - Mailing list pgsql-hackers

From Anton A. Melnikov
Subject Re: [BUG] pg_upgrade test fails from older versions.
Date
Msg-id f9d65c82-ebef-4ff8-1055-b1b704a19995@inbox.ru
Whole thread Raw
In response to Re: [BUG] pg_upgrade test fails from older versions.  (Michael Paquier <michael@paquier.xyz>)
Responses Re: [BUG] pg_upgrade test fails from older versions.  (Michael Paquier <michael@paquier.xyz>)
List 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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Error-safe user functions
Next
From: Amit Kapila
Date:
Subject: Re: Force streaming every change in logical decoding