Hi!

On 04.04.2025 19:32, Fujii Masao wrote:
I'm not sure there's clear consensus yet on the changes in the 0001 and
0002 patches, and it might not be worth rushing them in right before
the feature freeze. So for now, I reviewed and updated only the 0003 patch,
since there seems to be agreement on that one.

I've attached the updated version. It fixes a typo and replaces
the remaining hardcoded control file name with the XLOG_CONTROL_FILE macro.

Thanks! Looks good for me.
Maybe extend this to perl tests since there are several hardcoded names too?
The patch attached tries to do this.

How about committing the attached patch first? We can consider applying
the others later if consensus is reached

Yes, of cause. IMO, this is the best way now.


Best wishes,


--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From 370137af0dba390da825a4c556f7dc91e6c6b00a Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melni...@postgrespro.ru>
Date: Sat, 5 Apr 2025 01:38:45 +0300
Subject: [PATCH] Use XLOG_CONTROL_FILE macro in TAP tests.

---
 .../pg_controldata/t/001_pg_controldata.pl    |  2 +-
 src/bin/pg_resetwal/t/002_corrupted.pl        |  2 +-
 src/bin/pg_verifybackup/t/003_corruption.pl   |  3 ++-
 src/test/perl/PostgreSQL/Test/Cluster.pm      | 21 ++++++++++++++++++-
 src/test/recovery/t/042_low_level_backup.pl   | 11 +++++-----
 5 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl
index 4aea00d6d5a..7412c76e837 100644
--- a/src/bin/pg_controldata/t/001_pg_controldata.pl
+++ b/src/bin/pg_controldata/t/001_pg_controldata.pl
@@ -23,7 +23,7 @@ command_like([ 'pg_controldata', $node->data_dir ],
 
 # check with a corrupted pg_control
 
-my $pg_control = $node->data_dir . '/global/pg_control';
+my $pg_control = $node->data_dir . '/' . $node->controlfile;
 my $size = -s $pg_control;
 
 open my $fh, '>', $pg_control or BAIL_OUT($!);
diff --git a/src/bin/pg_resetwal/t/002_corrupted.pl b/src/bin/pg_resetwal/t/002_corrupted.pl
index 3c80f6309c9..3bf08030443 100644
--- a/src/bin/pg_resetwal/t/002_corrupted.pl
+++ b/src/bin/pg_resetwal/t/002_corrupted.pl
@@ -13,7 +13,7 @@ use Test::More;
 my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
 
-my $pg_control = $node->data_dir . '/global/pg_control';
+my $pg_control = $node->data_dir . '/' . $node->controlfile;
 my $size = -s $pg_control;
 
 # Read out the head of the file to get PG_CONTROL_VERSION in
diff --git a/src/bin/pg_verifybackup/t/003_corruption.pl b/src/bin/pg_verifybackup/t/003_corruption.pl
index 84f23b8bc3d..af39d57fbc6 100644
--- a/src/bin/pg_verifybackup/t/003_corruption.pl
+++ b/src/bin/pg_verifybackup/t/003_corruption.pl
@@ -23,6 +23,7 @@ $primary->start;
 my $source_ts_path = PostgreSQL::Test::Utils::tempdir_short();
 my $source_ts_prefix = $source_ts_path;
 $source_ts_prefix =~ s!(^[A-Z]:/[^/]*)/.*!$1!;
+my $controlfile = $primary->controlfile;
 
 $primary->safe_psql('postgres', <<EOM);
 CREATE TABLE x1 (a int);
@@ -280,7 +281,7 @@ sub mutilate_missing_tablespace
 sub mutilate_append_to_file
 {
 	my ($backup_path) = @_;
-	append_to_file "$backup_path/global/pg_control", 'x';
+	append_to_file "$backup_path/$controlfile", 'x';
 	return;
 }
 
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 8759ed2cbba..17ff8ce8fa3 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -263,6 +263,20 @@ sub logfile
 
 =pod
 
+=item $node->controlfile()
+
+Path to the PostgreSQL control file for this instance.
+
+=cut
+
+sub controlfile
+{
+	my ($self) = @_;
+	return $self->{_controlfile};
+}
+
+=pod
+
 =item $node->connstr()
 
 Get a libpq connection string that will establish a connection to
@@ -1595,6 +1609,10 @@ sub new
 		}
 	}
 
+	my @scan_result = PostgreSQL::Test::Utils::scan_server_header(
+						'access/xlog_internal.h',
+						'#define\s+XLOG_CONTROL_FILE\s+"(.*)"');
+	my $controlfile = $scan_result[0];
 	my $testname = basename($0);
 	$testname =~ s/\.[^.]+$//;
 	my $node = {
@@ -1607,7 +1625,8 @@ sub new
 		_logfile_base =>
 		  "$PostgreSQL::Test::Utils::log_path/${testname}_${name}",
 		_logfile =>
-		  "$PostgreSQL::Test::Utils::log_path/${testname}_${name}.log"
+		  "$PostgreSQL::Test::Utils::log_path/${testname}_${name}.log",
+		_controlfile => $controlfile
 	};
 
 	if ($params{install_path})
diff --git a/src/test/recovery/t/042_low_level_backup.pl b/src/test/recovery/t/042_low_level_backup.pl
index 5749a1df533..e2962536b2b 100644
--- a/src/test/recovery/t/042_low_level_backup.pl
+++ b/src/test/recovery/t/042_low_level_backup.pl
@@ -41,8 +41,9 @@ unlink("$backup_dir/postmaster.pid")
   or BAIL_OUT("unable to unlink $backup_dir/postmaster.pid");
 unlink("$backup_dir/postmaster.opts")
   or BAIL_OUT("unable to unlink $backup_dir/postmaster.opts");
-unlink("$backup_dir/global/pg_control")
-  or BAIL_OUT("unable to unlink $backup_dir/global/pg_control");
+my $controlfile = $node_primary->controlfile;
+unlink("$backup_dir/$controlfile")
+  or BAIL_OUT("unable to unlink $backup_dir/$controlfile");
 
 rmtree("$backup_dir/pg_wal")
   or BAIL_OUT("unable to unlink contents of $backup_dir/pg_wal");
@@ -69,9 +70,9 @@ $node_primary->poll_query_until('postgres',
 $node_primary->safe_psql('postgres', "checkpoint");
 
 # Copy pg_control last so it contains the new checkpoint.
-copy($node_primary->data_dir . '/global/pg_control',
-	"$backup_dir/global/pg_control")
-  or BAIL_OUT("unable to copy global/pg_control");
+copy($node_primary->data_dir . '/' . $controlfile,
+	"$backup_dir/$controlfile")
+  or BAIL_OUT("unable to copy $controlfile");
 
 # Save the name segment that will be archived by pg_backup_stop().
 # This is copied to the pg_wal directory of the node whose recovery
-- 
2.48.1

Reply via email to