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
commit f4a6f18f008ab4acd0d6923ddce7aa6d20bef08f
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date:   Thu Dec 22 07:22:53 2022 +0300

    Remove type 'aclitem' from old dump when test upgrade from older versions.

diff --git a/src/bin/pg_upgrade/upgrade_adapt.sql b/src/bin/pg_upgrade/upgrade_adapt.sql
index 27c4c7fd01..84389f3e5b 100644
--- a/src/bin/pg_upgrade/upgrade_adapt.sql
+++ b/src/bin/pg_upgrade/upgrade_adapt.sql
@@ -19,7 +19,8 @@ SELECT
   ver <= 906 AS oldpgversion_le96,
   ver <= 1000 AS oldpgversion_le10,
   ver <= 1100 AS oldpgversion_le11,
-  ver <= 1300 AS oldpgversion_le13
+  ver <= 1300 AS oldpgversion_le13,
+  ver <= 1500 AS oldpgversion_le15
   FROM (SELECT current_setting('server_version_num')::int / 100 AS ver) AS v;
 \gset
 
@@ -72,7 +73,7 @@ DO $stmt$
     FROM pg_class
     WHERE relname !~ '^pg_'
       AND relhasoids
-      AND relkind in ('r','m')
+      AND relkind = 'r'
     ORDER BY 1
   LOOP
     execute 'ALTER TABLE ' || rec || ' SET WITHOUT OIDS';
@@ -89,3 +90,58 @@ DROP OPERATOR public.#%# (pg_catalog.int8, NONE);
 DROP OPERATOR public.!=- (pg_catalog.int8, NONE);
 DROP OPERATOR public.#@%# (pg_catalog.int8, NONE);
 \endif
+
+-- The internal format of "aclitem" changed in PostgreSQL version 16
+-- so replace it with text type in tables and drop materialized views
+-- and indexes that contain column(s) of "aclitem" type.
+\if :oldpgversion_le15
+DO $$
+  DECLARE
+    rec text;
+	col text;
+  BEGIN
+  FOR rec in
+    SELECT oid::regclass::text
+    FROM pg_class
+    WHERE relname !~ '^pg_'
+      AND relkind = 'r'
+    ORDER BY 1
+  LOOP
+	FOR col in
+		SELECT attname
+		FROM pg_attribute
+		WHERE attrelid::regclass::text = rec
+		  AND atttypid = 'aclitem'::regtype
+	LOOP
+		execute 'ALTER TABLE ' || rec || ' ALTER COLUMN '
+							   || col || ' SET DATA TYPE text';
+	END LOOP;
+  END LOOP;
+  END; $$;
+
+DO $$
+  DECLARE
+    obj text;
+  BEGIN
+  FOR obj in
+	SELECT type || ' ' || name AS obj
+	FROM (SELECT
+			CASE
+				WHEN relkind = 'm' THEN 'MATERIALIZED VIEW'
+				WHEN relkind = 'i' THEN 'INDEX'
+			END AS type,
+			oid::regclass::text AS name
+		  FROM pg_class
+		  WHERE relname !~ '^pg_'
+			AND relkind in ('m','i')
+			AND EXISTS
+				(SELECT attname
+				 FROM pg_attribute
+				 WHERE attrelid = oid
+				   AND atttypid = 'aclitem'::regtype)
+		  ) AS foo
+  LOOP
+    execute 'DROP '|| obj || ' CASCADE';
+  END LOOP;
+  END; $$;
+\endif
commit 036e9d372241e7046e7b83baef991325d39730c3
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date:   Thu Dec 22 08:01:40 2022 +0300

    Additional dumps filtering during upgrade test.

diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING
index 127dc30bbb..8309aaeb05 100644
--- a/src/bin/pg_upgrade/TESTING
+++ b/src/bin/pg_upgrade/TESTING
@@ -56,3 +56,8 @@ Once the dump is created, it can be repeatedly used with $olddump and
 the dump out of the new database and the comparison of the dumps between
 the old and new databases.  The contents of the dumps can also be manually
 compared.
+
+You can use additional dump filtering. To do this, you need to define the
+'filter' environment variable and specify the path to the file with
+filtering rules in it. There is an example of such a file in
+src/bin/pg_upgrade/filter/regex.
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 4cc1469306..027b3fa35c 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -44,6 +44,33 @@ sub filter_dump
 	$dump_contents =~ s/^\-\-.*//mgx;
 	# Remove empty lines.
 	$dump_contents =~ s/^\n//mgx;
+	# Replace specific privilegies with ALL
+	$dump_contents =~ s/^(GRANT\s|REVOKE\s)(\S*)\s/$1ALL /mgx;
+
+	if (defined($ENV{filter}))
+	{
+		# Use the external filter
+		my $ext_filter_file = $ENV{filter};
+		die "no file with external filter found!" unless -e $ext_filter_file;
+
+		open my $ext_filter_handle, '<', $ext_filter_file;
+		chomp(my @ext_filter_lines = <$ext_filter_handle>);
+		close $ext_filter_handle;
+
+		foreach (@ext_filter_lines)
+		{
+			my @ext_filter = split('\/', $_);
+
+			if (defined $ext_filter[1] && defined $ext_filter[2])
+			{
+				$dump_contents =~ s/$ext_filter[1]/$ext_filter[2]/mgx;
+			}
+			elsif (defined $ext_filter[1])
+			{
+				$dump_contents =~ s/$ext_filter[1]//mgx;
+			}
+		}
+	}
 
 	my $dump_file_filtered = "${dump_file}_filtered";
 	open(my $dh, '>', $dump_file_filtered)
diff --git a/src/bin/pg_upgrade/t/filter.regex b/src/bin/pg_upgrade/t/filter.regex
new file mode 100644
index 0000000000..8b4c06c5f3
--- /dev/null
+++ b/src/bin/pg_upgrade/t/filter.regex
@@ -0,0 +1,5 @@
+# examples:
+# Remove all CREATE POLICY statements
+/^CREATE\sPOLICY.*//
+# Replace REFRESH with DROP for materialized views
+/^REFRESH\s(MATERIALIZED\sVIEW)/DROP $1/

Reply via email to