Hi all,
(Adding Andrew Dunstan in CC.)

I have been toying with $subject, trying to improve the ways to test
pg_upgrade across different major versions as perl makes that easier.
The buildfarm does three things to allow such tests to work (see
TestUpgradeXversion.pm):
- Apply a filter to the dumps generated to make them perfectly equal
as the set of regexps on the SQL dumps (removal of empty lines and
comments in the SQL dumps as arguments of a diff command).
- Apply diffs dumps to remove or modify objects, to avoid
inconsistencies, which is what upgrade_adapt.sql does in the tree.
- Add tweaks to the dump commands used, like --extra-float-digits=0
when testing with a version <= 11 as origin (aka c6f9464b in the
buildfarm client).

Attached is a basic patch to show what can be used to improve the TAP
tests of pg_upgrade in this area, with contents coming mostly from the
buildfarm client.  The end picture would be to allow all those tests
to use the core code, rather than duplicating that in the buildfarm
client.  This reduces a lot the amount of noise that can be seen when
comparing the dumps taken (the tests pass with v14) while remaining
simple, down to v11, so that could be a first step.  There are a
couple of things where I am not sure how the buildfarm handles things,
but perhaps the dumps of installcheck have been tweaked to ignore such
cases?  Here is an exhaustive list:
- multirange_type_name when using PG <= v13 as origin, for CREATE
TYPE.
- CREATE/ALTER PROCEDURE append IN to the list of parameters dumped,
when using PG <= v13 as origin.
- CREATE OPERATOR CLASS and ALTER OPERATOR FAMILY, where FUNCTION 2 is
moved from one command to the other.

Thoughts?
--
Michael
From a0876368a15a980e57e057481b47be8dd35cdbf9 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 24 May 2022 15:00:00 +0900
Subject: [PATCH] Add more filtering capabilities in the dumps.

This adds support for some filtering capabilities, for some tests done
down to v11.  This allows the tests to pass with v14, while v11~13 still
generate a few diffs.
---
 src/bin/pg_upgrade/t/002_pg_upgrade.pl | 80 ++++++++++++++++++++------
 1 file changed, 63 insertions(+), 17 deletions(-)

diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 75ac768a96..3d9b443048 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -26,6 +26,39 @@ sub generate_db
 	$node->command_ok([ 'createdb', $dbname ]);
 }
 
+# Filter the contents of a dump before its use in a content comparison.
+# This returns the path to the filtered dump.
+sub filter_dump
+{
+	my ($node, $tempdir, $dump_file_name) = @_;
+	my $dump_file     = "$tempdir/$dump_file_name";
+	my $dump_contents = slurp_file($dump_file);
+
+	# Remove the comments.
+	$dump_contents =~ s/^\-\-.*//mgx;
+	# Remove empty lines.
+	$dump_contents =~ s/^\n//mgx;
+
+	# Locale setup has changed for collations in 15~.
+	$dump_contents =~
+	  s/(^CREATE\sCOLLATION\s.*?)\slocale\s=\s'und'/$1 locale = ''/mgx
+	  if ($node->pg_version < 15);
+
+	# Dumps taken from <= 11 use EXECUTE PROCEDURE.  Replace it
+	# with EXECUTE FUNCTION.
+	$dump_contents =~
+	  s/(^CREATE\sTRIGGER\s.*?)\sEXECUTE\sPROCEDURE/$1 EXECUTE FUNCTION/mgx
+	  if ($node->pg_version < 12);
+
+	my $dump_file_filtered = "$tempdir/${dump_file_name}_filter";
+	open(my $dh, '>', $dump_file_filtered)
+	  || die "opening $dump_file_filtered";
+	print $dh $dump_contents;
+	close($dh);
+
+	return $dump_file_filtered;
+}
+
 # The test of pg_upgrade requires two clusters, an old one and a new one
 # that gets upgraded.  Before running the upgrade, a logical dump of the
 # old cluster is taken, and a second logical dump of the new one is taken
@@ -134,7 +167,7 @@ if (defined($ENV{oldinstall}))
 	$oldnode->command_ok(
 		[
 			'psql', '-X',
-			'-f', "$srcdir/src/bin/pg_upgrade/upgrade_adapt.sql",
+			'-f',   "$srcdir/src/bin/pg_upgrade/upgrade_adapt.sql",
 			'regression'
 		]);
 }
@@ -147,13 +180,13 @@ my $oldbindir = $oldnode->config_data('--bindir');
 
 # Take a dump before performing the upgrade as a base comparison. Note
 # that we need to use pg_dumpall from the new node here.
-$newnode->command_ok(
-	[
-		'pg_dumpall', '--no-sync',
-		'-d',         $oldnode->connstr('postgres'),
-		'-f',         "$tempdir/dump1.sql"
-	],
-	'dump before running pg_upgrade');
+my @dump_command = (
+	'pg_dumpall', '--no-sync', '-d', $oldnode->connstr('postgres'),
+	'-f',         "$tempdir/dump1.sql");
+# --extra-float-digits is needed when upgrading from a version older than 11.
+push(@dump_command, '--extra-float-digits', '0')
+  if ($oldnode->pg_version < 12);
+$newnode->command_ok(\@dump_command, 'dump before running pg_upgrade');
 
 # After dumping, update references to the old source tree's regress.so
 # to point to the new tree.
@@ -228,23 +261,36 @@ if (-d $log_path)
 }
 
 # Second dump from the upgraded instance.
-$newnode->command_ok(
-	[
-		'pg_dumpall', '--no-sync',
-		'-d',         $newnode->connstr('postgres'),
-		'-f',         "$tempdir/dump2.sql"
-	]);
+# that we need to use pg_dumpall from the new node here.
+@dump_command = (
+	'pg_dumpall', '--no-sync', '-d', $newnode->connstr('postgres'),
+	'-f',         "$tempdir/dump2.sql");
+# --extra-float-digits is needed when upgrading from a version older than 11.
+push(@dump_command, '--extra-float-digits', '0')
+  if ($oldnode->pg_version < 12);
+$newnode->command_ok(\@dump_command, 'dump after running pg_upgrade');
+
+# Filter the contents of the dumps from the old version of any contents.
+my $dump1_filtered = "$tempdir/dump1.sql";
+my $dump2_filtered = "$tempdir/dump2.sql";
+
+# No need to apply filters on the dumps if working on the same version.
+if ($oldnode->pg_version != $newnode->pg_version)
+{
+	$dump1_filtered = filter_dump($oldnode, $tempdir, "dump1.sql");
+	$dump2_filtered = filter_dump($newnode, $tempdir, "dump2.sql");
+}
 
 # Compare the two dumps, there should be no differences.
-my $compare_res = compare("$tempdir/dump1.sql", "$tempdir/dump2.sql");
+my $compare_res = compare($dump1_filtered, $dump2_filtered);
 is($compare_res, 0, 'old and new dumps match after pg_upgrade');
 
 # Provide more context if the dumps do not match.
 if ($compare_res != 0)
 {
 	my ($stdout, $stderr) =
-	  run_command([ 'diff', "$tempdir/dump1.sql", "$tempdir/dump2.sql" ]);
-	print "=== diff of $tempdir/dump1.sql and $tempdir/dump2.sql\n";
+	  run_command([ 'diff', $dump1_filtered, $dump2_filtered ]);
+	print "=== diff of $dump1_filtered and $dump2_filtered\n";
 	print "=== stdout ===\n";
 	print $stdout;
 	print "=== stderr ===\n";
-- 
2.36.1

Attachment: signature.asc
Description: PGP signature

Reply via email to