0002-Address-comments-by-Daniel-Gustafsson-20241220.patch

From 74f9a88c6f7ddfe26019dbd50f98c2789029ad9f Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: Fri, 20 Dec 2024 15:22:14 +0530
Subject: [PATCH 2/2] Address comments by Daniel Gustafsson

To be merged with the earlier commit.
---
 doc/src/sgml/regress.sgml                   |  3 ++-
 src/bin/pg_upgrade/t/002_pg_upgrade.pl      | 14 ++++++--------
 src/test/perl/PostgreSQL/Test/AdjustDump.pm |  2 +-
 src/test/perl/PostgreSQL/Test/Utils.pm      |  7 +++++--
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 4be5d2d7d52..60da8eb95e5 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -343,7 +343,8 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption'
       <para>
        When enabled, <filename>src/bin/pg_upgrade/t/002_pg_upgrade.pl</filename>
        tests dump and restore of regression database left behind by the
-       regression run. Not enabled by default because it is time consuming.
+       regression run. Not enabled by default because it is time and resource
+       consuming.
       </para>
      </listitem>
     </varlistentry>
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 42b68527146..a817ed0d00b 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -38,7 +38,7 @@ sub generate_db
 
 # Filter the contents of a dump before its use in a content comparison for
 # upgrade testing. This returns the path to the filtered dump.
-sub filter_dump_for_upgrade
+sub filter_dump
 {
 	my ($is_old, $old_version, $dump_file) = @_;
 	my $dump_contents = slurp_file($dump_file);
@@ -525,10 +525,8 @@ push(@dump_command, '--extra-float-digits', '0')
 $newnode->command_ok(\@dump_command, 'dump after running pg_upgrade');
 
 # Filter the contents of the dumps.
-my $dump1_filtered =
-  filter_dump_for_upgrade(1, $oldnode->pg_version, $dump1_file);
-my $dump2_filtered =
-  filter_dump_for_upgrade(0, $oldnode->pg_version, $dump2_file);
+my $dump1_filtered = filter_dump(1, $oldnode->pg_version, $dump1_file);
+my $dump2_filtered = filter_dump(0, $oldnode->pg_version, $dump2_file);
 
 # Compare the two dumps, there should be no differences.
 compare_dumps($dump1_filtered, $dump2_filtered,
@@ -545,6 +543,7 @@ sub test_regression_dump_restore
 {
 	my ($src_node, %node_params) = @_;
 	my $dst_node = PostgreSQL::Test::Cluster->new('dst_node');
+	my %dump_formats = ('plain' => 'p', 'tar' => 't', 'directory' => 'd', 'custom' => 'c');
 
 	# Dump the original database for comparison later.
 	my $src_dump = get_dump_for_comparison($src_node->connstr('regression'),
@@ -554,10 +553,9 @@ sub test_regression_dump_restore
 	$dst_node->init(%node_params);
 	$dst_node->start;
 
-	for my $format ('plain', 'tar', 'directory', 'custom')
+	while (my ($format, $format_spec) = each %dump_formats)
 	{
 		my $dump_file = "$tempdir/regression_dump.$format";
-		my $format_spec = substr($format, 0, 1);
 		my $restored_db = 'regression_' . $format;
 
 		# Even though we compare only schema from the original and the restored
@@ -628,7 +626,7 @@ sub get_dump_for_comparison
 		'dump for comparison succeeded');
 
 	open(my $dh, '>', $dump_adjusted)
-	  || die "opening $dump_adjusted ";
+	  || die "could not open $dump_adjusted for writing the adjusted dump: $!";
 	print $dh adjust_regress_dumpfile(slurp_file($dumpfile),
 		$adjust_child_columns);
 	close($dh);
diff --git a/src/test/perl/PostgreSQL/Test/AdjustDump.pm b/src/test/perl/PostgreSQL/Test/AdjustDump.pm
index 0b0abb0cefc..5b9990e4719 100644
--- a/src/test/perl/PostgreSQL/Test/AdjustDump.pm
+++ b/src/test/perl/PostgreSQL/Test/AdjustDump.pm
@@ -18,7 +18,7 @@ PostgreSQL::Test::AdjustDump - helper module for dump and restore tests
 =head1 DESCRIPTION
 
 C<PostgreSQL::Test::AdjustDump> encapsulates various hacks needed to
-compare the results of dump and retore tests
+compare the results of dump and restore tests
 
 =cut
 
diff --git a/src/test/perl/PostgreSQL/Test/Utils.pm b/src/test/perl/PostgreSQL/Test/Utils.pm
index 6efe5faf77d..bf56eb4b23c 100644
--- a/src/test/perl/PostgreSQL/Test/Utils.pm
+++ b/src/test/perl/PostgreSQL/Test/Utils.pm
@@ -1120,8 +1120,11 @@ sub compare_dumps
 	}
 	else
 	{
-		note('first dump file: ' . $dump1);
-		note('second dump file: ' . $dump2);
+		# Fail if the comparison succeeds because the files are the same. This
+		# will detect simple programming errors. It won't detect more complex
+		# errors like passing different links pointing to the same underlying
+		# file.
+		ok($dump1 ne $dump2, "dump files being compared are distinct")
 	}
 
 	return;
-- 
2.34.1