[PATCH v8 4/8] Propagate changes to indisclustered to child/parents

public inbox for [email protected]  
help / color / mirror / Atom feed

[PATCH v8 4/8] Propagate changes to indisclustered to child/parents
7+ messages / 4 participants
[nested] [flat]

* [PATCH v8 4/8] Propagate changes to indisclustered to child/parents
@ 2020-10-07 03:11  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Justin Pryzby @ 2020-10-07 03:11 UTC (permalink / raw)

---
 src/backend/commands/cluster.c        | 109 ++++++++++++++++----------
 src/backend/commands/indexcmds.c      |   2 +
 src/test/regress/expected/cluster.out |  46 +++++++++++
 src/test/regress/sql/cluster.sql      |  11 +++
 4 files changed, 125 insertions(+), 43 deletions(-)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index cb4fc350c6..5c08f0642e 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -74,6 +74,7 @@ static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose);
 static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
 							bool verbose, bool *pSwapToastByContent,
 							TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
+static void set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index);
 static List *get_tables_to_cluster(MemoryContext cluster_context);
 static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
 		Oid indexOid);
@@ -516,66 +517,88 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
 	index_close(OldIndex, NoLock);
 }
 
+/*
+ * Helper for mark_index_clustered
+ * Mark a single index as clustered or not.
+ * pg_index is passed by caller to avoid repeatedly re-opening it.
+ */
+static void
+set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index)
+{
+	HeapTuple	indexTuple;
+	Form_pg_index indexForm;
+
+	indexTuple = SearchSysCacheCopy1(INDEXRELID,
+									ObjectIdGetDatum(indexOid));
+	if (!HeapTupleIsValid(indexTuple))
+		elog(ERROR, "cache lookup failed for index %u", indexOid);
+	indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
+	/* this was checked earlier, but let's be real sure */
+	if (isclustered && !indexForm->indisvalid)
+		elog(ERROR, "cannot cluster on invalid index %u", indexOid);
+
+	indexForm->indisclustered = isclustered;
+	CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
+	heap_freetuple(indexTuple);
+}
+
 /*
  * mark_index_clustered: mark the specified index as the one clustered on
  *
- * With indexOid == InvalidOid, will mark all indexes of rel not-clustered.
+ * With indexOid == InvalidOid, mark all indexes of rel not-clustered.
+ * Otherwise, mark children of the clustered index as clustered, and parents of
+ * other indexes as unclustered.
+ * We wish to maintain the following properties:
+ * 1) Only one index on a relation can be marked clustered at once
+ * 2) If a partitioned index is clustered, then all its children must be
+ *     clustered.
  */
 void
 mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
 {
-	HeapTuple	indexTuple;
-	Form_pg_index indexForm;
-	Relation	pg_index;
-	ListCell   *index;
-
-	/*
-	 * If the index is already marked clustered, no need to do anything.
-	 */
-	if (OidIsValid(indexOid))
-	{
-		if (get_index_isclustered(indexOid))
-			return;
-	}
+	ListCell	*lc, *lc2;
+	List		*indexes;
+	Relation	pg_index = table_open(IndexRelationId, RowExclusiveLock);
+	List		*inh = find_all_inheritors(RelationGetRelid(rel), ShareRowExclusiveLock, NULL);
 
 	/*
 	 * Check each index of the relation and set/clear the bit as needed.
+	 * Iterate over the relation's children rather than the index's children
+	 * since we need to unset cluster for indexes on intermediate children,
+	 * too.
 	 */
-	pg_index = table_open(IndexRelationId, RowExclusiveLock);
-
-	foreach(index, RelationGetIndexList(rel))
+	foreach(lc, inh)
 	{
-		Oid			thisIndexOid = lfirst_oid(index);
-
-		indexTuple = SearchSysCacheCopy1(INDEXRELID,
-										 ObjectIdGetDatum(thisIndexOid));
-		if (!HeapTupleIsValid(indexTuple))
-			elog(ERROR, "cache lookup failed for index %u", thisIndexOid);
-		indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+		Oid			inhrelid = lfirst_oid(lc);
+		Relation	thisrel = table_open(inhrelid, ShareRowExclusiveLock);
 
-		/*
-		 * Unset the bit if set.  We know it's wrong because we checked this
-		 * earlier.
-		 */
-		if (indexForm->indisclustered)
+		indexes = RelationGetIndexList(thisrel);
+		foreach (lc2, indexes)
 		{
-			indexForm->indisclustered = false;
-			CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
-		}
-		else if (thisIndexOid == indexOid)
-		{
-			/* this was checked earlier, but let's be real sure */
-			if (!indexForm->indisvalid)
-				elog(ERROR, "cannot cluster on invalid index %u", indexOid);
-			indexForm->indisclustered = true;
-			CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
-		}
+			bool	isclustered;
+			Oid		thisIndexOid = lfirst_oid(lc2);
+			List	*parentoids = get_rel_relispartition(thisIndexOid) ?
+				get_partition_ancestors(thisIndexOid) : NIL;
 
-		InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
-									 InvalidOid, is_internal);
+			/*
+			 * A child of the clustered index must be set clustered;
+			 * indexes which are not children of the clustered index are
+			 * set unclustered
+			 */
+			isclustered = (thisIndexOid == indexOid) ||
+					list_member_oid(parentoids, indexOid);
+			Assert(OidIsValid(indexOid) || !isclustered);
+			set_indisclustered(thisIndexOid, isclustered, pg_index);
+
+			InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
+										 InvalidOid, is_internal);
+		}
 
-		heap_freetuple(indexTuple);
+		list_free(indexes);
+		table_close(thisrel, ShareRowExclusiveLock);
 	}
+	list_free(inh);
 
 	table_close(pg_index, RowExclusiveLock);
 }
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 127ba7835d..4ca1ffbfa4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -25,6 +25,7 @@
 #include "catalog/catalog.h"
 #include "catalog/index.h"
 #include "catalog/indexing.h"
+#include "catalog/partition.h"
 #include "catalog/pg_am.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_inherits.h"
@@ -32,6 +33,7 @@
 #include "catalog/pg_opfamily.h"
 #include "catalog/pg_tablespace.h"
 #include "catalog/pg_type.h"
+#include "commands/cluster.h"
 #include "commands/comment.h"
 #include "commands/dbcommands.h"
 #include "commands/defrem.h"
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index c74cfa88cc..1d436dfaae 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -495,6 +495,52 @@ Indexes:
     "clstrpart_idx" btree (a) CLUSTER
 Number of partitions: 3 (Use \d+ to list them.)
 
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+            Table "public.clstrpart33"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ a      | integer |           |          | 
+Partition of: clstrpart3 DEFAULT
+Indexes:
+    "clstrpart33_a_idx" btree (a) CLUSTER
+
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+            Table "public.clstrpart33"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ a      | integer |           |          | 
+Partition of: clstrpart3 DEFAULT
+Indexes:
+    "clstrpart33_a_idx" btree (a)
+
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+            Table "public.clstrpart33"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ a      | integer |           |          | 
+Partition of: clstrpart3 DEFAULT
+Indexes:
+    "clstrpart33_a_idx" btree (a) CLUSTER
+
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
+       Partitioned table "public.clstrpart1"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ a      | integer |           |          | 
+Partition of: clstrpart FOR VALUES FROM (1) TO (10)
+Partition key: RANGE (a)
+Indexes:
+    "clstrpart1_a_idx" btree (a)
+    "clstrpart1_idx_2" btree (a) CLUSTER
+Number of partitions: 2 (Use \d+ to list them.)
+
 -- Test CLUSTER with external tuplesorting
 create table clstr_4 as select * from tenk1;
 create index cluster_sort on clstr_4 (hundred, thousand, tenthous);
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index 9bcc77695c..0ded2be1ca 100644
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -220,6 +220,17 @@ CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitio
 CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs
 CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf
 \d clstrpart
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
 
 -- Test CLUSTER with external tuplesorting
 
-- 
2.17.0


--O5XBE6gyVG5Rl6Rj
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v8-0005-Invalidate-parent-indexes.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
@ 2022-01-05 08:12  Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2022-01-05 08:12 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Postgres hackers <[email protected]>

On Thu, Dec 16, 2021 at 11:51:55AM +0900, Michael Paquier wrote:
> I may have missed one thing or two, but I think that's pretty much
> what we should be looking for to do the switch to TAP in terms of
> coverage.

Rebased patch to cool down the CF bot, as per the addition of
--no-sync to pg_upgrade.
--
Michael


Attachments:

  [text/x-diff] v5-0001-Switch-tests-of-pg_upgrade-to-use-TAP.patch (28.2K, ../../[email protected]/2-v5-0001-Switch-tests-of-pg_upgrade-to-use-TAP.patch)
  download | inline diff:
From f7af03ec5e5c6a685796d58b6d82eb9f603565de Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Wed, 5 Jan 2022 17:11:12 +0900
Subject: [PATCH v5] Switch tests of pg_upgrade to use TAP

---
 src/bin/pg_upgrade/.gitignore            |   5 +
 src/bin/pg_upgrade/Makefile              |  23 +-
 src/bin/pg_upgrade/TESTING               |  33 ++-
 src/bin/pg_upgrade/t/001_basic.pl        |   9 +
 src/bin/pg_upgrade/t/002_pg_upgrade.pl   | 275 ++++++++++++++++++++++
 src/bin/pg_upgrade/test.sh               | 279 -----------------------
 src/test/perl/PostgreSQL/Test/Cluster.pm |  25 ++
 src/tools/msvc/vcregress.pl              |  94 +-------
 8 files changed, 355 insertions(+), 388 deletions(-)
 create mode 100644 src/bin/pg_upgrade/t/001_basic.pl
 create mode 100644 src/bin/pg_upgrade/t/002_pg_upgrade.pl
 delete mode 100644 src/bin/pg_upgrade/test.sh

diff --git a/src/bin/pg_upgrade/.gitignore b/src/bin/pg_upgrade/.gitignore
index 2d3bfeaa50..3b64522ab6 100644
--- a/src/bin/pg_upgrade/.gitignore
+++ b/src/bin/pg_upgrade/.gitignore
@@ -7,3 +7,8 @@
 /loadable_libraries.txt
 /log/
 /tmp_check/
+
+# Generated by pg_regress
+/sql/
+/expected/
+/results/
diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile
index 44d06be5a6..35b6c123a5 100644
--- a/src/bin/pg_upgrade/Makefile
+++ b/src/bin/pg_upgrade/Makefile
@@ -28,6 +28,12 @@ OBJS = \
 override CPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
 LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
 
+# required for 002_pg_upgrade.pl
+REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
+export REGRESS_SHLIB
+REGRESS_OUTPUTDIR=$(abs_top_builddir)/src/bin/pg_upgrade
+export REGRESS_OUTPUTDIR
+
 all: pg_upgrade
 
 pg_upgrade: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
@@ -49,17 +55,8 @@ clean distclean maintainer-clean:
 	       pg_upgrade_dump_globals.sql \
 	       pg_upgrade_dump_*.custom pg_upgrade_*.log
 
-# When $(MAKE) is present, make automatically infers that this is a
-# recursive make. which is not actually what we want here, as that
-# e.g. prevents output synchronization from working (as make thinks
-# that the subsidiary make knows how to deal with that itself, but
-# we're invoking a shell script that doesn't know). Referencing
-# $(MAKE) indirectly avoids that behaviour.
-# See https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html#MAKE-Variable
-NOTSUBMAKEMAKE=$(MAKE)
+check:
+	$(prove_check)
 
-check: test.sh all temp-install
-	MAKE=$(NOTSUBMAKEMAKE) $(with_temp_install) bindir=$(abs_top_builddir)/tmp_install/$(bindir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $<
-
-# installcheck is not supported because there's no meaningful way to test
-# pg_upgrade against a single already-running server
+installcheck:
+	$(prove_installcheck)
diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING
index 78b9747908..43a71566e2 100644
--- a/src/bin/pg_upgrade/TESTING
+++ b/src/bin/pg_upgrade/TESTING
@@ -2,21 +2,30 @@ THE SHORT VERSION
 -----------------
 
 On non-Windows machines, you can execute the testing process
-described below by running
+described below by running the following command in this directory:
 	make check
-in this directory.  This will run the shell script test.sh, performing
-an upgrade from the version in this source tree to a new instance of
-the same version.
 
-To test an upgrade from a different version, you must have a built
-source tree for the old version as well as this version, and you
-must have done "make install" for both versions.  Then do:
+This will run the TAP tests to run pg_upgrade, performing an upgrade
+from the version in this source tree to a  new instance of the same
+version.
+
+To test an upgrade from a different version, there are two options
+available:
+
+1) You have a built source tree for the old version as well as this
+version's binaries.  Then set up the following variables before
+launching the test:
 
 export oldsrc=...somewhere/postgresql	(old version's source tree)
-export oldbindir=...otherversion/bin	(old version's installed bin dir)
-export bindir=...thisversion/bin	(this version's installed bin dir)
-export libdir=...thisversion/lib	(this version's installed lib dir)
-sh test.sh
+export oldinstall=...otherversion/	(old version's install base path)
+
+2) You have a dump that can be used to set up the old version, as well
+as this version's binaries.  Then set up the following variables:
+export olddump=...somewhere/dump.sql	(old version's dump)
+export oldinstall=...otherversion/	(old version's install base path)
+
+Finally, the tests can be done by running
+	make check
 
 In this case, you will have to manually eyeball the resulting dump
 diff for version-specific differences, as explained below.
@@ -77,3 +86,5 @@ steps:
 
 7)  Diff the regression database dump file with the regression dump
     file loaded into the old server.
+
+The generated dump may be reusable with "olddump", as defined above.
diff --git a/src/bin/pg_upgrade/t/001_basic.pl b/src/bin/pg_upgrade/t/001_basic.pl
new file mode 100644
index 0000000000..75b0f98b08
--- /dev/null
+++ b/src/bin/pg_upgrade/t/001_basic.pl
@@ -0,0 +1,9 @@
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More tests => 8;
+
+program_help_ok('pg_upgrade');
+program_version_ok('pg_upgrade');
+program_options_handling_ok('pg_upgrade');
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
new file mode 100644
index 0000000000..17ba2d3cd2
--- /dev/null
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -0,0 +1,275 @@
+# Set of tests for pg_upgrade, including cross-version checks.
+use strict;
+use warnings;
+
+use Cwd qw(abs_path getcwd);
+use File::Basename qw(dirname);
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 4;
+
+# Generate a database with a name made of a range of ASCII characters.
+sub generate_db
+{
+	my ($node, $from_char, $to_char) = @_;
+
+	my $dbname = '';
+	for my $i ($from_char .. $to_char)
+	{
+		next if $i == 7 || $i == 10 || $i == 13;    # skip BEL, LF, and CR
+		$dbname = $dbname . sprintf('%c', $i);
+	}
+	$node->run_log([ 'createdb', '--port', $node->port, $dbname ]);
+}
+
+# From now on, the test of pg_upgrade consists in setting up an instance.
+# This is the source instance used for the upgrade. Then a new and fresh
+# instance is created, and is used as the target instance for the
+# upgrade.  Before running an upgrade a logical dump of the old instance
+# is taken, and a second logical dump of the new instance is taken after
+# the upgrade.  The upgrade test passes if there are no differences after
+# running pg_upgrade.
+
+# Testing upgrades with an older instance of PostgreSQL requires
+# setting up two environment variables, among the following:
+# - "oldsrc", to point to the code source of the older version.
+#   This is required to set up the old instance with pg_upgrade.
+# - "olddump", to point to a dump file that will be used to set
+#   up the old instance to upgrade from.
+# - "oldinstall", to point to the installation path of the older
+# version.
+
+# "oldsrc" and "olddump" cannot be used together.  Setting up
+# "olddump" and "oldinstall" will use the dump pointed to to
+# set up the old instance.  If "oldsrc" is used instead of "olddump",
+# the full set of regression tests of the old instance is run
+# instead.
+
+if (defined($ENV{oldsrc}) && defined($ENV{olddump}))
+{
+	die "oldsrc and olddump are both defined";
+}
+elsif (defined($ENV{oldsrc}))
+{
+	if (   (defined($ENV{oldsrc}) && !defined($ENV{oldinstall}))
+		|| (!defined($ENV{oldsrc}) && defined($ENV{oldinstall})))
+	{
+		# Not all variables are defined, so leave and die if test is
+		# done with an older installation.
+		die "oldsrc or oldinstall is undefined";
+	}
+}
+elsif (defined($ENV{olddump}))
+{
+	if (   (defined($ENV{olddump}) && !defined($ENV{oldinstall}))
+		|| (!defined($ENV{olddump}) && defined($ENV{oldinstall})))
+	{
+		# Not all variables are defined, so leave and die if test is
+		# done with an older installation.
+		die "olddump or oldinstall is undefined";
+	}
+}
+
+if ((defined($ENV{oldsrc}) || defined($ENV{olddump})) && $windows_os)
+{
+	# This configuration is not supported on Windows, as regress.so
+	# location diverges across the compilation methods used on this
+	# platform.
+	die "No support for older version tests on Windows";
+}
+
+# Default is the location of this source code for both nodes used with
+# the upgrade.
+my $newsrc = abs_path("../../..");
+my $oldsrc = $ENV{oldsrc} || $newsrc;
+$oldsrc = abs_path($oldsrc);
+
+# Temporary location for the dumps taken
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+
+# Initialize node to upgrade
+my $oldnode = PostgreSQL::Test::Cluster->new('old_node',
+	install_path => $ENV{oldinstall});
+
+$oldnode->init(extra => [ '--locale', 'C', '--encoding', 'LATIN1' ]);
+$oldnode->start;
+
+# Set up the data of the old instance with pg_regress or an old dump.
+if (defined($ENV{olddump}))
+{
+	# Use the dump specified.
+	my $olddumpfile = $ENV{olddump};
+	die "no dump file found!" unless -e $olddumpfile;
+
+	# Load the dump, and we are done here.
+	$oldnode->command_ok(
+		[
+			'psql',   '-X',           '-f', $olddumpfile,
+			'--port', $oldnode->port, 'regression'
+		]);
+}
+else
+{
+	# Default is to just use pg_regress to setup the old instance
+	# Creating databases with names covering most ASCII bytes
+	generate_db($oldnode, 1,  45);
+	generate_db($oldnode, 46, 90);
+	generate_db($oldnode, 91, 127);
+
+	# Run core regression tests on the old instance.
+	$oldnode->run_log([ "createdb", '--port', $oldnode->port, 'regression' ]);
+
+	# Grab any regression options that may be passed down by caller.
+	my $extra_opts_val = $ENV{EXTRA_REGRESS_OPT} || "";
+	my @extra_opts     = split(/\s+/, $extra_opts_val);
+
+	# --dlpath is needed to be able to find the location of regress.so and
+	# any libraries the regression tests required.  This needs to point to
+	# in the old instance when using it.  In the default case, fallback to
+	# what the caller provided for REGRESS_SHLIB.
+	my $dlpath;
+	if (defined($ENV{oldinstall}))
+	{
+		$dlpath = "$oldsrc/src/test/regress";
+	}
+	else
+	{
+		$dlpath = dirname($ENV{REGRESS_SHLIB});
+	}
+	$dlpath = PostgreSQL::Test::Utils::perl2host($dlpath);
+
+	# --outputdir points to the path where to place the output files, which
+	# had better be this directory.
+	my $outputdir =
+	  PostgreSQL::Test::Utils::perl2host($ENV{REGRESS_OUTPUTDIR});
+
+	# --inputdir needs to point to the location of the input files, from the
+	# cluster to-be-upgraded.
+	my $inputdir =
+	  PostgreSQL::Test::Utils::perl2host("$oldsrc/src/test/regress");
+
+	my @regress_command = [
+		$ENV{PG_REGRESS}, '--make-testtablespace-dir',
+		'--schedule',     "$oldsrc/src/test/regress/parallel_schedule",
+		'--bindir',       $oldnode->config_data('--bindir'),
+		'--dlpath',       $dlpath,
+		'--port',         $oldnode->port,
+		'--outputdir',    $outputdir,
+		'--inputdir',     $inputdir,
+		'--use-existing'
+	];
+	@regress_command = (@regress_command, @extra_opts);
+
+	$oldnode->command_ok(@regress_command,
+		'regression test run on old instance');
+}
+
+# Before dumping, get rid of objects not existing or not supported in later
+# versions. This depends on the version of the old server used, and matters
+# only if different versions are used for the dump.
+if (defined($ENV{oldinstall}))
+{
+	# Note that upgrade_adapt.sql from the new version is used, to
+	# cope with an upgrade to this version.
+	$oldnode->run_log(
+		[
+			'psql', '-X', '-f',
+			PostgreSQL::Test::Utils::perl2host(
+				"$newsrc/src/bin/pg_upgrade/upgrade_adapt.sql"),
+			'--port',
+			$oldnode->port,
+			'regression'
+		]);
+}
+
+# Initialize a new node for the upgrade.  This is done early so as it is
+# possible to know with which node's PATH the initial dump needs to be
+# taken.
+my $newnode = PostgreSQL::Test::Cluster->new('new_node');
+$newnode->init(extra => [ '--locale=C', '--encoding=LATIN1' ]);
+my $newbindir = $newnode->config_data('--bindir');
+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');
+
+# After dumping, update references to the old source tree's regress.so
+# to point to the new tree.
+if (defined($ENV{oldinstall}))
+{
+	# First, fetch all the references to libraries that are not part
+	# of the default path $libdir.
+	my $output = $oldnode->safe_psql('regression',
+		"SELECT DISTINCT probin::text FROM pg_proc WHERE probin NOT LIKE '\$libdir%';"
+	);
+	chomp($output);
+	my @libpaths = split("\n", $output);
+
+	my $dump_data = slurp_file("$tempdir/dump1.sql");
+
+	my $newregresssrc = "$newsrc/src/test/regress";
+	foreach (@libpaths)
+	{
+		my $libpath = $_;
+		$libpath = dirname($libpath);
+		$dump_data =~ s/$libpath/$newregresssrc/g;
+	}
+
+	open my $fh, ">", "$tempdir/dump1.sql" or die "could not open dump file";
+	print $fh $dump_data;
+	close $fh;
+
+	# This replaces any references to the old tree's regress.so
+	# the new tree's regress.so.  Any references that do *not*
+	# match $libdir are switched so as this request does not
+	# depend on the path of the old source tree.  This is useful
+	# when using an old dump.  Do the operation on all the database
+	# that allow connections so as this includes the regression
+	# database and anything the user has set up.
+	$output = $oldnode->safe_psql('postgres',
+		"SELECT datname FROM pg_database WHERE datallowconn;");
+	chomp($output);
+	my @datnames = split("\n", $output);
+	foreach (@datnames)
+	{
+		my $datname = $_;
+		$oldnode->safe_psql(
+			$datname, "UPDATE pg_proc SET probin =
+		  regexp_replace(probin, '.*/', '$newregresssrc/')
+		  WHERE probin NOT LIKE '\$libdir/%'");
+	}
+}
+
+# Update the instance.
+$oldnode->stop;
+
+# Time for the real run.
+$newnode->command_ok(
+	[
+		'pg_upgrade', '--no-sync',        '-d', $oldnode->data_dir,
+		'-D',         $newnode->data_dir, '-b', $oldbindir,
+		'-B',         $newbindir,         '-p', $oldnode->port,
+		'-P',         $newnode->port,
+	],
+	'run of pg_upgrade for new instance');
+$newnode->start;
+
+# Take a second dump on the upgraded instance.
+$newnode->run_log(
+	[
+		'pg_dumpall', '--no-sync',
+		'-d',         $newnode->connstr('postgres'),
+		'-f',         "$tempdir/dump2.sql"
+	]);
+
+# Compare the two dumps, there should be no differences.
+command_ok([ 'diff', '-q', "$tempdir/dump1.sql", "$tempdir/dump2.sql" ],
+	'old and new dump match after pg_upgrade');
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
deleted file mode 100644
index d6a318367a..0000000000
--- a/src/bin/pg_upgrade/test.sh
+++ /dev/null
@@ -1,279 +0,0 @@
-#!/bin/sh
-
-# src/bin/pg_upgrade/test.sh
-#
-# Test driver for pg_upgrade.  Initializes a new database cluster,
-# runs the regression tests (to put in some data), runs pg_dumpall,
-# runs pg_upgrade, runs pg_dumpall again, compares the dumps.
-#
-# Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
-# Portions Copyright (c) 1994, Regents of the University of California
-
-set -e
-
-: ${MAKE=make}
-
-# Guard against parallel make issues (see comments in pg_regress.c)
-unset MAKEFLAGS
-unset MAKELEVEL
-
-# Run a given "initdb" binary and overlay the regression testing
-# authentication configuration.
-standard_initdb() {
-	# To increase coverage of non-standard segment size and group access
-	# without increasing test runtime, run these tests with a custom setting.
-	# Also, specify "-A trust" explicitly to suppress initdb's warning.
-	# --allow-group-access and --wal-segsize have been added in v11.
-	"$1" -N --wal-segsize 1 --allow-group-access -A trust
-	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
-	then
-		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
-	fi
-	../../test/regress/pg_regress --config-auth "$PGDATA"
-}
-
-# What flavor of host are we on?
-# Treat MINGW* (msys1) and MSYS* (msys2) the same.
-testhost=`uname -s | sed 's/^MSYS/MINGW/'`
-
-# Establish how the server will listen for connections
-case $testhost in
-	MINGW*)
-		LISTEN_ADDRESSES="localhost"
-		PG_REGRESS_SOCKET_DIR=""
-		PGHOST=localhost
-		;;
-	*)
-		LISTEN_ADDRESSES=""
-		# Select a socket directory.  The algorithm is from the "configure"
-		# script; the outcome mimics pg_regress.c:make_temp_sockdir().
-		if [ x"$PG_REGRESS_SOCKET_DIR" = x ]; then
-			set +e
-			dir=`(umask 077 &&
-				  mktemp -d /tmp/pg_upgrade_check-XXXXXX) 2>/dev/null`
-			if [ ! -d "$dir" ]; then
-				dir=/tmp/pg_upgrade_check-$$-$RANDOM
-				(umask 077 && mkdir "$dir")
-				if [ ! -d "$dir" ]; then
-					echo "could not create socket temporary directory in \"/tmp\""
-					exit 1
-				fi
-			fi
-			set -e
-			PG_REGRESS_SOCKET_DIR=$dir
-			trap 'rm -rf "$PG_REGRESS_SOCKET_DIR"' 0
-			trap 'exit 3' 1 2 13 15
-		fi
-		PGHOST=$PG_REGRESS_SOCKET_DIR
-		;;
-esac
-
-POSTMASTER_OPTS="-F -c listen_addresses=\"$LISTEN_ADDRESSES\" -k \"$PG_REGRESS_SOCKET_DIR\""
-export PGHOST
-
-# don't rely on $PWD here, as old shells don't set it
-temp_root=`pwd`/tmp_check
-rm -rf "$temp_root"
-mkdir "$temp_root"
-
-: ${oldbindir=$bindir}
-
-: ${oldsrc=../../..}
-oldsrc=`cd "$oldsrc" && pwd`
-newsrc=`cd ../../.. && pwd`
-
-# We need to make pg_regress use psql from the desired installation
-# (likely a temporary one), because otherwise the installcheck run
-# below would try to use psql from the proper installation directory
-# of the target version, which might be outdated or not exist. But
-# don't override anything else that's already in EXTRA_REGRESS_OPTS.
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --bindir='$oldbindir'"
-export EXTRA_REGRESS_OPTS
-
-# While in normal cases this will already be set up, adding bindir to
-# path allows test.sh to be invoked with different versions as
-# described in ./TESTING
-PATH=$bindir:$PATH
-export PATH
-
-BASE_PGDATA="$temp_root/data"
-PGDATA="${BASE_PGDATA}.old"
-export PGDATA
-
-# Send installcheck outputs to a private directory.  This avoids conflict when
-# check-world runs pg_upgrade check concurrently with src/test/regress check.
-# To retrieve interesting files after a run, use pattern tmp_check/*/*.diffs.
-outputdir="$temp_root/regress"
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"
-export EXTRA_REGRESS_OPTS
-mkdir "$outputdir"
-
-# pg_regress --make-tablespacedir would take care of that in 14~, but this is
-# still required for older versions where this option is not supported.
-if [ "$newsrc" != "$oldsrc" ]; then
-	mkdir "$outputdir"/testtablespace
-	mkdir "$outputdir"/sql
-	mkdir "$outputdir"/expected
-fi
-
-logdir=`pwd`/log
-rm -rf "$logdir"
-mkdir "$logdir"
-
-# Clear out any environment vars that might cause libpq to connect to
-# the wrong postmaster (cf pg_regress.c)
-#
-# Some shells, such as NetBSD's, return non-zero from unset if the variable
-# is already unset. Since we are operating under 'set -e', this causes the
-# script to fail. To guard against this, set them all to an empty string first.
-PGDATABASE="";        unset PGDATABASE
-PGUSER="";            unset PGUSER
-PGSERVICE="";         unset PGSERVICE
-PGSSLMODE="";         unset PGSSLMODE
-PGREQUIRESSL="";      unset PGREQUIRESSL
-PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT
-PGHOSTADDR="";        unset PGHOSTADDR
-
-# Select a non-conflicting port number, similarly to pg_regress.c
-PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$newsrc"/src/include/pg_config.h | awk '{print $3}'`
-PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
-export PGPORT
-
-i=0
-while psql -X postgres </dev/null 2>/dev/null
-do
-	i=`expr $i + 1`
-	if [ $i -eq 16 ]
-	then
-		echo port $PGPORT apparently in use
-		exit 1
-	fi
-	PGPORT=`expr $PGPORT + 1`
-	export PGPORT
-done
-
-# buildfarm may try to override port via EXTRA_REGRESS_OPTS ...
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --port=$PGPORT"
-export EXTRA_REGRESS_OPTS
-
-standard_initdb "$oldbindir"/initdb
-"$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
-
-# Create databases with names covering the ASCII bytes other than NUL, BEL,
-# LF, or CR.  BEL would ring the terminal bell in the course of this test, and
-# it is not otherwise a special case.  PostgreSQL doesn't support the rest.
-dbname1=`awk 'BEGIN { for (i= 1; i < 46; i++)
-	if (i != 7 && i != 10 && i != 13) printf "%c", i }' </dev/null`
-# Exercise backslashes adjacent to double quotes, a Windows special case.
-dbname1='\"\'$dbname1'\\"\\\'
-dbname2=`awk 'BEGIN { for (i = 46; i <  91; i++) printf "%c", i }' </dev/null`
-dbname3=`awk 'BEGIN { for (i = 91; i < 128; i++) printf "%c", i }' </dev/null`
-createdb "regression$dbname1" || createdb_status=$?
-createdb "regression$dbname2" || createdb_status=$?
-createdb "regression$dbname3" || createdb_status=$?
-
-# Extra options to apply to the dump.  This may be changed later.
-extra_dump_options=""
-
-if "$MAKE" -C "$oldsrc" installcheck-parallel; then
-	oldpgversion=`psql -X -A -t -d regression -c "SHOW server_version_num"`
-
-	# Before dumping, tweak the database of the old instance depending
-	# on its version.
-	if [ "$newsrc" != "$oldsrc" ]; then
-		# This SQL script has its own idea of the cleanup that needs to be
-		# done on the cluster to-be-upgraded, and includes version checks.
-		# Note that this uses the script stored on the new branch.
-		psql -X -d regression -f "$newsrc/src/bin/pg_upgrade/upgrade_adapt.sql" \
-			|| psql_fix_sql_status=$?
-
-		# Handling of --extra-float-digits gets messy after v12.
-		# Note that this changes the dumps from the old and new
-		# instances if involving an old cluster of v11 or older.
-		if [ $oldpgversion -lt 120000 ]; then
-			extra_dump_options="--extra-float-digits=0"
-		fi
-	fi
-
-	pg_dumpall $extra_dump_options --no-sync \
-		-f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
-
-	if [ "$newsrc" != "$oldsrc" ]; then
-		# update references to old source tree's regress.so etc
-		fix_sql=""
-		case $oldpgversion in
-			*)
-				fix_sql="UPDATE pg_proc SET probin = replace(probin, '$oldsrc', '$newsrc') WHERE probin LIKE '$oldsrc%';"
-				;;
-		esac
-		psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-
-		mv "$temp_root"/dump1.sql "$temp_root"/dump1.sql.orig
-		sed "s;$oldsrc;$newsrc;g" "$temp_root"/dump1.sql.orig >"$temp_root"/dump1.sql
-	fi
-else
-	make_installcheck_status=$?
-fi
-"$oldbindir"/pg_ctl -m fast stop
-if [ -n "$createdb_status" ]; then
-	exit 1
-fi
-if [ -n "$make_installcheck_status" ]; then
-	exit 1
-fi
-if [ -n "$psql_fix_sql_status" ]; then
-	exit 1
-fi
-if [ -n "$pg_dumpall1_status" ]; then
-	echo "pg_dumpall of pre-upgrade database cluster failed"
-	exit 1
-fi
-
-PGDATA="$BASE_PGDATA"
-
-standard_initdb 'initdb'
-
-pg_upgrade $PG_UPGRADE_OPTS --no-sync -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -p "$PGPORT" -P "$PGPORT"
-
-# make sure all directories and files have group permissions, on Unix hosts
-# Windows hosts don't support Unix-y permissions.
-case $testhost in
-	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type f ! -perm 640 | wc -l` -ne 0 ]; then
-			echo "files in PGDATA with permission != 640";
-			exit 1;
-		fi ;;
-esac
-
-case $testhost in
-	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type d ! -perm 750 | wc -l` -ne 0 ]; then
-			echo "directories in PGDATA with permission != 750";
-			exit 1;
-		fi ;;
-esac
-
-pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w
-
-pg_dumpall $extra_dump_options --no-sync \
-	-f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
-pg_ctl -m fast stop
-
-if [ -n "$pg_dumpall2_status" ]; then
-	echo "pg_dumpall of post-upgrade database cluster failed"
-	exit 1
-fi
-
-case $testhost in
-	MINGW*)	MSYS2_ARG_CONV_EXCL=/c cmd /c delete_old_cluster.bat ;;
-	*)	    sh ./delete_old_cluster.sh ;;
-esac
-
-if diff "$temp_root"/dump1.sql "$temp_root"/dump2.sql >/dev/null; then
-	echo PASSED
-	exit 0
-else
-	echo "Files $temp_root/dump1.sql and $temp_root/dump2.sql differ"
-	echo "dumps were not identical"
-	exit 1
-fi
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index c061e850fb..2bf2fcb69d 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -327,6 +327,31 @@ sub install_path
 
 =pod
 
+=item $node->config_data($option)
+
+Grab some data from pg_config, with $option being the switched
+used.
+
+=cut
+
+sub config_data
+{
+	my ($self, $option) = @_;
+	local %ENV = $self->_get_env();
+
+	my ($stdout, $stderr);
+	my $result =
+	  IPC::Run::run [ $self->installed_command('pg_config'), $option ],
+	  '>', \$stdout, '2>', \$stderr
+	  or die "could not execute pg_config";
+	chomp($stdout);
+	$stdout =~ s/\r$//;
+
+	return $stdout;
+}
+
+=pod
+
 =item $node->info()
 
 Return a string containing human-readable diagnostic information (paths, etc)
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 29086cab51..d25810373a 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -287,6 +287,10 @@ sub bincheck
 	foreach my $dir (@bin_dirs)
 	{
 		next unless -d "$dir/t";
+		# Do not consider pg_upgrade, as it is handled by
+		# upgradecheck.
+		next if ($dir =~ "/pg_upgrade/");
+
 		my $status = tap_check($dir);
 		$mstat ||= $status;
 	}
@@ -591,91 +595,11 @@ sub generate_db
 
 sub upgradecheck
 {
-	my $status;
-	my $cwd = getcwd();
-
-	# Much of this comes from the pg_upgrade test.sh script,
-	# but it only covers the --install case, and not the case
-	# where the old and new source or bin dirs are different.
-	# i.e. only this version to this version check. That's
-	# what pg_upgrade's "make check" does.
-
-	$ENV{PGHOST} = 'localhost';
-	$ENV{PGPORT} ||= 50432;
-	my $tmp_root = "$topdir/src/bin/pg_upgrade/tmp_check";
-	rmtree($tmp_root);
-	mkdir $tmp_root || die $!;
-	my $upg_tmp_install = "$tmp_root/install";    # unshared temp install
-	print "Setting up temp install\n\n";
-	Install($upg_tmp_install, "all", $config);
-
-	# Install does a chdir, so change back after that
-	chdir $cwd;
-	my ($bindir, $libdir, $oldsrc, $newsrc) =
-	  ("$upg_tmp_install/bin", "$upg_tmp_install/lib", $topdir, $topdir);
-	$ENV{PATH} = "$bindir;$ENV{PATH}";
-	my $data = "$tmp_root/data";
-	$ENV{PGDATA} = "$data.old";
-	my $outputdir          = "$tmp_root/regress";
-	my @EXTRA_REGRESS_OPTS = ("--outputdir=$outputdir");
-	mkdir "$outputdir" || die $!;
-
-	my $logdir = "$topdir/src/bin/pg_upgrade/log";
-	rmtree($logdir);
-	mkdir $logdir || die $!;
-	print "\nRunning initdb on old cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nStarting old cluster\n\n";
-	my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log");
-	system(@args) == 0 or exit 1;
-
-	print "\nCreating databases with names covering most ASCII bytes\n\n";
-	generate_db("\\\"\\", 1,  45,  "\\\\\"\\\\\\");
-	generate_db('',       46, 90,  '');
-	generate_db('',       91, 127, '');
-
-	print "\nSetting up data for upgrading\n\n";
-	installcheck_internal('parallel', @EXTRA_REGRESS_OPTS);
-
-	# now we can chdir into the source dir
-	chdir "$topdir/src/bin/pg_upgrade";
-	print "\nDumping old cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump1.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping old cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	$ENV{PGDATA} = "$data";
-	print "\nSetting up new cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nRunning pg_upgrade\n\n";
-	@args = (
-		'pg_upgrade', '-d', "$data.old", '-D', $data, '-b', $bindir,
-		'--no-sync');
-	system(@args) == 0 or exit 1;
-	print "\nStarting new cluster\n\n";
-	@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", 'start');
-	system(@args) == 0 or exit 1;
-	print "\nDumping new cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump2.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping new cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	print "\nDeleting old cluster\n\n";
-	system(".\\delete_old_cluster.bat") == 0 or exit 1;
-	print "\nComparing old and new cluster dumps\n\n";
-
-	@args = ('diff', '-q', "$tmp_root/dump1.sql", "$tmp_root/dump2.sql");
-	system(@args);
-	$status = $?;
-	if (!$status)
-	{
-		print "PASSED\n";
-	}
-	else
-	{
-		print "dumps not identical!\n";
-		exit(1);
-	}
+	InstallTemp();
+	# Tweak environment for the upgrade tests
+	$ENV{REGRESS_OUTPUTDIR} = "$topdir/src/bin/pg_upgrade";
+	my $mstat = tap_check("$topdir/src/bin/pg_upgrade");
+	exit $mstat if $mstat;
 	return;
 }
 
-- 
2.34.1



  [application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
@ 2022-01-11 07:14  Michael Paquier <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2022-01-11 07:14 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Postgres hackers <[email protected]>

On Wed, Jan 05, 2022 at 05:12:41PM +0900, Michael Paquier wrote:
> Rebased patch to cool down the CF bot, as per the addition of
> --no-sync to pg_upgrade.

The CF bot is unhappy, so here is a rebase, with some typo fixes
reported by Justin offlist.
--
Michael


Attachments:

  [text/x-diff] v6-0001-Switch-tests-of-pg_upgrade-to-use-TAP.patch (28.2K, ../../[email protected]/2-v6-0001-Switch-tests-of-pg_upgrade-to-use-TAP.patch)
  download | inline diff:
From 1d2dc68b8c27d9c328276ed21e57290f64bca586 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Tue, 11 Jan 2022 16:13:02 +0900
Subject: [PATCH v6] Switch tests of pg_upgrade to use TAP

---
 src/bin/pg_upgrade/.gitignore            |   5 +
 src/bin/pg_upgrade/Makefile              |  23 +-
 src/bin/pg_upgrade/TESTING               |  33 ++-
 src/bin/pg_upgrade/t/001_basic.pl        |   9 +
 src/bin/pg_upgrade/t/002_pg_upgrade.pl   | 275 ++++++++++++++++++++++
 src/bin/pg_upgrade/test.sh               | 279 -----------------------
 src/test/perl/PostgreSQL/Test/Cluster.pm |  25 ++
 src/tools/msvc/vcregress.pl              |  94 +-------
 8 files changed, 355 insertions(+), 388 deletions(-)
 create mode 100644 src/bin/pg_upgrade/t/001_basic.pl
 create mode 100644 src/bin/pg_upgrade/t/002_pg_upgrade.pl
 delete mode 100644 src/bin/pg_upgrade/test.sh

diff --git a/src/bin/pg_upgrade/.gitignore b/src/bin/pg_upgrade/.gitignore
index 2d3bfeaa50..3b64522ab6 100644
--- a/src/bin/pg_upgrade/.gitignore
+++ b/src/bin/pg_upgrade/.gitignore
@@ -7,3 +7,8 @@
 /loadable_libraries.txt
 /log/
 /tmp_check/
+
+# Generated by pg_regress
+/sql/
+/expected/
+/results/
diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile
index 44d06be5a6..35b6c123a5 100644
--- a/src/bin/pg_upgrade/Makefile
+++ b/src/bin/pg_upgrade/Makefile
@@ -28,6 +28,12 @@ OBJS = \
 override CPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
 LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
 
+# required for 002_pg_upgrade.pl
+REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
+export REGRESS_SHLIB
+REGRESS_OUTPUTDIR=$(abs_top_builddir)/src/bin/pg_upgrade
+export REGRESS_OUTPUTDIR
+
 all: pg_upgrade
 
 pg_upgrade: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
@@ -49,17 +55,8 @@ clean distclean maintainer-clean:
 	       pg_upgrade_dump_globals.sql \
 	       pg_upgrade_dump_*.custom pg_upgrade_*.log
 
-# When $(MAKE) is present, make automatically infers that this is a
-# recursive make. which is not actually what we want here, as that
-# e.g. prevents output synchronization from working (as make thinks
-# that the subsidiary make knows how to deal with that itself, but
-# we're invoking a shell script that doesn't know). Referencing
-# $(MAKE) indirectly avoids that behaviour.
-# See https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html#MAKE-Variable
-NOTSUBMAKEMAKE=$(MAKE)
+check:
+	$(prove_check)
 
-check: test.sh all temp-install
-	MAKE=$(NOTSUBMAKEMAKE) $(with_temp_install) bindir=$(abs_top_builddir)/tmp_install/$(bindir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $<
-
-# installcheck is not supported because there's no meaningful way to test
-# pg_upgrade against a single already-running server
+installcheck:
+	$(prove_installcheck)
diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING
index 78b9747908..43a71566e2 100644
--- a/src/bin/pg_upgrade/TESTING
+++ b/src/bin/pg_upgrade/TESTING
@@ -2,21 +2,30 @@ THE SHORT VERSION
 -----------------
 
 On non-Windows machines, you can execute the testing process
-described below by running
+described below by running the following command in this directory:
 	make check
-in this directory.  This will run the shell script test.sh, performing
-an upgrade from the version in this source tree to a new instance of
-the same version.
 
-To test an upgrade from a different version, you must have a built
-source tree for the old version as well as this version, and you
-must have done "make install" for both versions.  Then do:
+This will run the TAP tests to run pg_upgrade, performing an upgrade
+from the version in this source tree to a  new instance of the same
+version.
+
+To test an upgrade from a different version, there are two options
+available:
+
+1) You have a built source tree for the old version as well as this
+version's binaries.  Then set up the following variables before
+launching the test:
 
 export oldsrc=...somewhere/postgresql	(old version's source tree)
-export oldbindir=...otherversion/bin	(old version's installed bin dir)
-export bindir=...thisversion/bin	(this version's installed bin dir)
-export libdir=...thisversion/lib	(this version's installed lib dir)
-sh test.sh
+export oldinstall=...otherversion/	(old version's install base path)
+
+2) You have a dump that can be used to set up the old version, as well
+as this version's binaries.  Then set up the following variables:
+export olddump=...somewhere/dump.sql	(old version's dump)
+export oldinstall=...otherversion/	(old version's install base path)
+
+Finally, the tests can be done by running
+	make check
 
 In this case, you will have to manually eyeball the resulting dump
 diff for version-specific differences, as explained below.
@@ -77,3 +86,5 @@ steps:
 
 7)  Diff the regression database dump file with the regression dump
     file loaded into the old server.
+
+The generated dump may be reusable with "olddump", as defined above.
diff --git a/src/bin/pg_upgrade/t/001_basic.pl b/src/bin/pg_upgrade/t/001_basic.pl
new file mode 100644
index 0000000000..75b0f98b08
--- /dev/null
+++ b/src/bin/pg_upgrade/t/001_basic.pl
@@ -0,0 +1,9 @@
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More tests => 8;
+
+program_help_ok('pg_upgrade');
+program_version_ok('pg_upgrade');
+program_options_handling_ok('pg_upgrade');
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
new file mode 100644
index 0000000000..e66879c6ea
--- /dev/null
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -0,0 +1,275 @@
+# Set of tests for pg_upgrade, including cross-version checks.
+use strict;
+use warnings;
+
+use Cwd qw(abs_path getcwd);
+use File::Basename qw(dirname);
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 4;
+
+# Generate a database with a name made of a range of ASCII characters.
+sub generate_db
+{
+	my ($node, $from_char, $to_char) = @_;
+
+	my $dbname = '';
+	for my $i ($from_char .. $to_char)
+	{
+		next if $i == 7 || $i == 10 || $i == 13;    # skip BEL, LF, and CR
+		$dbname = $dbname . sprintf('%c', $i);
+	}
+	$node->run_log([ 'createdb', '--port', $node->port, $dbname ]);
+}
+
+# From now on, the test of pg_upgrade consists in setting up an instance.
+# This is the source instance used for the upgrade. Then a new and fresh
+# instance is created, and is used as the target instance for the
+# upgrade.  Before running an upgrade, a logical dump of the old instance
+# is taken, and a second logical dump of the new instance is taken after
+# the upgrade.  The upgrade test passes if there are no differences after
+# running pg_upgrade.
+
+# Testing upgrades with an older instance of PostgreSQL requires
+# setting up two environment variables, among the following:
+# - "oldsrc", to point to the code source of the older version.
+#   This is required to set up the old instance with pg_upgrade.
+# - "olddump", to point to a dump file that will be used to set
+#   up the old instance to upgrade from.
+# - "oldinstall", to point to the installation path of the older
+# version.
+
+# "oldsrc" and "olddump" cannot be used together.  Setting up
+# "olddump" and "oldinstall" will use the dump pointed to to
+# set up the old instance.  If "oldsrc" is used instead of "olddump",
+# the full set of regression tests of the old instance is run
+# instead.
+
+if (defined($ENV{oldsrc}) && defined($ENV{olddump}))
+{
+	die "oldsrc and olddump are both defined";
+}
+elsif (defined($ENV{oldsrc}))
+{
+	if (   (defined($ENV{oldsrc}) && !defined($ENV{oldinstall}))
+		|| (!defined($ENV{oldsrc}) && defined($ENV{oldinstall})))
+	{
+		# Not all variables are defined, so leave and die if test is
+		# done with an older installation.
+		die "oldsrc or oldinstall is undefined";
+	}
+}
+elsif (defined($ENV{olddump}))
+{
+	if (   (defined($ENV{olddump}) && !defined($ENV{oldinstall}))
+		|| (!defined($ENV{olddump}) && defined($ENV{oldinstall})))
+	{
+		# Not all variables are defined, so leave and die if test is
+		# done with an older installation.
+		die "olddump or oldinstall is undefined";
+	}
+}
+
+if ((defined($ENV{oldsrc}) || defined($ENV{olddump})) && $windows_os)
+{
+	# This configuration is not supported on Windows, as regress.so
+	# location diverges across the compilation methods used on this
+	# platform.
+	die "No support for older version tests on Windows";
+}
+
+# Default is the location of this source code for both nodes used with
+# the upgrade.
+my $newsrc = abs_path("../../..");
+my $oldsrc = $ENV{oldsrc} || $newsrc;
+$oldsrc = abs_path($oldsrc);
+
+# Temporary location for the dumps taken
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+
+# Initialize node to upgrade
+my $oldnode = PostgreSQL::Test::Cluster->new('old_node',
+	install_path => $ENV{oldinstall});
+
+$oldnode->init(extra => [ '--locale', 'C', '--encoding', 'LATIN1' ]);
+$oldnode->start;
+
+# Set up the data of the old instance with pg_regress or an old dump.
+if (defined($ENV{olddump}))
+{
+	# Use the dump specified.
+	my $olddumpfile = $ENV{olddump};
+	die "no dump file found!" unless -e $olddumpfile;
+
+	# Load the dump, and we are done here.
+	$oldnode->command_ok(
+		[
+			'psql',   '-X',           '-f', $olddumpfile,
+			'--port', $oldnode->port, 'regression'
+		]);
+}
+else
+{
+	# Default is to just use pg_regress to set up the old instance
+	# Creating databases with names covering most ASCII bytes
+	generate_db($oldnode, 1,  45);
+	generate_db($oldnode, 46, 90);
+	generate_db($oldnode, 91, 127);
+
+	# Run core regression tests on the old instance.
+	$oldnode->run_log([ "createdb", '--port', $oldnode->port, 'regression' ]);
+
+	# Grab any regression options that may be passed down by caller.
+	my $extra_opts_val = $ENV{EXTRA_REGRESS_OPT} || "";
+	my @extra_opts     = split(/\s+/, $extra_opts_val);
+
+	# --dlpath is needed to be able to find the location of regress.so and
+	# any libraries the regression tests required.  This needs to point to
+	# the old instance when using it.  In the default case, fallback to
+	# what the caller provided for REGRESS_SHLIB.
+	my $dlpath;
+	if (defined($ENV{oldinstall}))
+	{
+		$dlpath = "$oldsrc/src/test/regress";
+	}
+	else
+	{
+		$dlpath = dirname($ENV{REGRESS_SHLIB});
+	}
+	$dlpath = PostgreSQL::Test::Utils::perl2host($dlpath);
+
+	# --outputdir points to the path where to place the output files, which
+	# had better be this directory.
+	my $outputdir =
+	  PostgreSQL::Test::Utils::perl2host($ENV{REGRESS_OUTPUTDIR});
+
+	# --inputdir needs to point to the location of the input files, from the
+	# cluster to-be-upgraded.
+	my $inputdir =
+	  PostgreSQL::Test::Utils::perl2host("$oldsrc/src/test/regress");
+
+	my @regress_command = [
+		$ENV{PG_REGRESS}, '--make-testtablespace-dir',
+		'--schedule',     "$oldsrc/src/test/regress/parallel_schedule",
+		'--bindir',       $oldnode->config_data('--bindir'),
+		'--dlpath',       $dlpath,
+		'--port',         $oldnode->port,
+		'--outputdir',    $outputdir,
+		'--inputdir',     $inputdir,
+		'--use-existing'
+	];
+	@regress_command = (@regress_command, @extra_opts);
+
+	$oldnode->command_ok(@regress_command,
+		'regression test run on old instance');
+}
+
+# Before dumping, get rid of objects not existing or not supported in later
+# versions. This depends on the version of the old server used, and matters
+# only if different versions are used for the dump.
+if (defined($ENV{oldinstall}))
+{
+	# Note that upgrade_adapt.sql from the new version is used, to
+	# cope with an upgrade to this version.
+	$oldnode->run_log(
+		[
+			'psql', '-X', '-f',
+			PostgreSQL::Test::Utils::perl2host(
+				"$newsrc/src/bin/pg_upgrade/upgrade_adapt.sql"),
+			'--port',
+			$oldnode->port,
+			'regression'
+		]);
+}
+
+# Initialize a new node for the upgrade.  This is done early so as it is
+# possible to know with which node's PATH the initial dump needs to be
+# taken.
+my $newnode = PostgreSQL::Test::Cluster->new('new_node');
+$newnode->init(extra => [ '--locale=C', '--encoding=LATIN1' ]);
+my $newbindir = $newnode->config_data('--bindir');
+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');
+
+# After dumping, update references to the old source tree's regress.so
+# to point to the new tree.
+if (defined($ENV{oldinstall}))
+{
+	# First, fetch all the references to libraries that are not part
+	# of the default path $libdir.
+	my $output = $oldnode->safe_psql('regression',
+		"SELECT DISTINCT probin::text FROM pg_proc WHERE probin NOT LIKE '\$libdir%';"
+	);
+	chomp($output);
+	my @libpaths = split("\n", $output);
+
+	my $dump_data = slurp_file("$tempdir/dump1.sql");
+
+	my $newregresssrc = "$newsrc/src/test/regress";
+	foreach (@libpaths)
+	{
+		my $libpath = $_;
+		$libpath = dirname($libpath);
+		$dump_data =~ s/$libpath/$newregresssrc/g;
+	}
+
+	open my $fh, ">", "$tempdir/dump1.sql" or die "could not open dump file";
+	print $fh $dump_data;
+	close $fh;
+
+	# This replaces any references to the old tree's regress.so
+	# the new tree's regress.so.  Any references that do *not*
+	# match $libdir are switched so as this request does not
+	# depend on the path of the old source tree.  This is useful
+	# when using an old dump.  Do the operation on all the databases
+	# that allow connections so as this includes the regression
+	# database and anything the user has set up.
+	$output = $oldnode->safe_psql('postgres',
+		"SELECT datname FROM pg_database WHERE datallowconn;");
+	chomp($output);
+	my @datnames = split("\n", $output);
+	foreach (@datnames)
+	{
+		my $datname = $_;
+		$oldnode->safe_psql(
+			$datname, "UPDATE pg_proc SET probin =
+		  regexp_replace(probin, '.*/', '$newregresssrc/')
+		  WHERE probin NOT LIKE '\$libdir/%'");
+	}
+}
+
+# Update the instance.
+$oldnode->stop;
+
+# Time for the real run.
+$newnode->command_ok(
+	[
+		'pg_upgrade', '--no-sync',        '-d', $oldnode->data_dir,
+		'-D',         $newnode->data_dir, '-b', $oldbindir,
+		'-B',         $newbindir,         '-p', $oldnode->port,
+		'-P',         $newnode->port,
+	],
+	'run of pg_upgrade for new instance');
+$newnode->start;
+
+# Take a second dump on the upgraded instance.
+$newnode->run_log(
+	[
+		'pg_dumpall', '--no-sync',
+		'-d',         $newnode->connstr('postgres'),
+		'-f',         "$tempdir/dump2.sql"
+	]);
+
+# Compare the two dumps, there should be no differences.
+command_ok([ 'diff', '-q', "$tempdir/dump1.sql", "$tempdir/dump2.sql" ],
+	'old and new dump match after pg_upgrade');
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
deleted file mode 100644
index ef328b3062..0000000000
--- a/src/bin/pg_upgrade/test.sh
+++ /dev/null
@@ -1,279 +0,0 @@
-#!/bin/sh
-
-# src/bin/pg_upgrade/test.sh
-#
-# Test driver for pg_upgrade.  Initializes a new database cluster,
-# runs the regression tests (to put in some data), runs pg_dumpall,
-# runs pg_upgrade, runs pg_dumpall again, compares the dumps.
-#
-# Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
-# Portions Copyright (c) 1994, Regents of the University of California
-
-set -e
-
-: ${MAKE=make}
-
-# Guard against parallel make issues (see comments in pg_regress.c)
-unset MAKEFLAGS
-unset MAKELEVEL
-
-# Run a given "initdb" binary and overlay the regression testing
-# authentication configuration.
-standard_initdb() {
-	# To increase coverage of non-standard segment size and group access
-	# without increasing test runtime, run these tests with a custom setting.
-	# Also, specify "-A trust" explicitly to suppress initdb's warning.
-	# --allow-group-access and --wal-segsize have been added in v11.
-	"$1" -N --wal-segsize 1 --allow-group-access -A trust
-	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
-	then
-		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
-	fi
-	../../test/regress/pg_regress --config-auth "$PGDATA"
-}
-
-# What flavor of host are we on?
-# Treat MINGW* (msys1) and MSYS* (msys2) the same.
-testhost=`uname -s | sed 's/^MSYS/MINGW/'`
-
-# Establish how the server will listen for connections
-case $testhost in
-	MINGW*)
-		LISTEN_ADDRESSES="localhost"
-		PG_REGRESS_SOCKET_DIR=""
-		PGHOST=localhost
-		;;
-	*)
-		LISTEN_ADDRESSES=""
-		# Select a socket directory.  The algorithm is from the "configure"
-		# script; the outcome mimics pg_regress.c:make_temp_sockdir().
-		if [ x"$PG_REGRESS_SOCKET_DIR" = x ]; then
-			set +e
-			dir=`(umask 077 &&
-				  mktemp -d /tmp/pg_upgrade_check-XXXXXX) 2>/dev/null`
-			if [ ! -d "$dir" ]; then
-				dir=/tmp/pg_upgrade_check-$$-$RANDOM
-				(umask 077 && mkdir "$dir")
-				if [ ! -d "$dir" ]; then
-					echo "could not create socket temporary directory in \"/tmp\""
-					exit 1
-				fi
-			fi
-			set -e
-			PG_REGRESS_SOCKET_DIR=$dir
-			trap 'rm -rf "$PG_REGRESS_SOCKET_DIR"' 0
-			trap 'exit 3' 1 2 13 15
-		fi
-		PGHOST=$PG_REGRESS_SOCKET_DIR
-		;;
-esac
-
-POSTMASTER_OPTS="-F -c listen_addresses=\"$LISTEN_ADDRESSES\" -k \"$PG_REGRESS_SOCKET_DIR\""
-export PGHOST
-
-# don't rely on $PWD here, as old shells don't set it
-temp_root=`pwd`/tmp_check
-rm -rf "$temp_root"
-mkdir "$temp_root"
-
-: ${oldbindir=$bindir}
-
-: ${oldsrc=../../..}
-oldsrc=`cd "$oldsrc" && pwd`
-newsrc=`cd ../../.. && pwd`
-
-# We need to make pg_regress use psql from the desired installation
-# (likely a temporary one), because otherwise the installcheck run
-# below would try to use psql from the proper installation directory
-# of the target version, which might be outdated or not exist. But
-# don't override anything else that's already in EXTRA_REGRESS_OPTS.
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --bindir='$oldbindir'"
-export EXTRA_REGRESS_OPTS
-
-# While in normal cases this will already be set up, adding bindir to
-# path allows test.sh to be invoked with different versions as
-# described in ./TESTING
-PATH=$bindir:$PATH
-export PATH
-
-BASE_PGDATA="$temp_root/data"
-PGDATA="${BASE_PGDATA}.old"
-export PGDATA
-
-# Send installcheck outputs to a private directory.  This avoids conflict when
-# check-world runs pg_upgrade check concurrently with src/test/regress check.
-# To retrieve interesting files after a run, use pattern tmp_check/*/*.diffs.
-outputdir="$temp_root/regress"
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"
-export EXTRA_REGRESS_OPTS
-mkdir "$outputdir"
-
-# pg_regress --make-tablespacedir would take care of that in 14~, but this is
-# still required for older versions where this option is not supported.
-if [ "$newsrc" != "$oldsrc" ]; then
-	mkdir "$outputdir"/testtablespace
-	mkdir "$outputdir"/sql
-	mkdir "$outputdir"/expected
-fi
-
-logdir=`pwd`/log
-rm -rf "$logdir"
-mkdir "$logdir"
-
-# Clear out any environment vars that might cause libpq to connect to
-# the wrong postmaster (cf pg_regress.c)
-#
-# Some shells, such as NetBSD's, return non-zero from unset if the variable
-# is already unset. Since we are operating under 'set -e', this causes the
-# script to fail. To guard against this, set them all to an empty string first.
-PGDATABASE="";        unset PGDATABASE
-PGUSER="";            unset PGUSER
-PGSERVICE="";         unset PGSERVICE
-PGSSLMODE="";         unset PGSSLMODE
-PGREQUIRESSL="";      unset PGREQUIRESSL
-PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT
-PGHOSTADDR="";        unset PGHOSTADDR
-
-# Select a non-conflicting port number, similarly to pg_regress.c
-PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$newsrc"/src/include/pg_config.h | awk '{print $3}'`
-PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
-export PGPORT
-
-i=0
-while psql -X postgres </dev/null 2>/dev/null
-do
-	i=`expr $i + 1`
-	if [ $i -eq 16 ]
-	then
-		echo port $PGPORT apparently in use
-		exit 1
-	fi
-	PGPORT=`expr $PGPORT + 1`
-	export PGPORT
-done
-
-# buildfarm may try to override port via EXTRA_REGRESS_OPTS ...
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --port=$PGPORT"
-export EXTRA_REGRESS_OPTS
-
-standard_initdb "$oldbindir"/initdb
-"$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
-
-# Create databases with names covering the ASCII bytes other than NUL, BEL,
-# LF, or CR.  BEL would ring the terminal bell in the course of this test, and
-# it is not otherwise a special case.  PostgreSQL doesn't support the rest.
-dbname1=`awk 'BEGIN { for (i= 1; i < 46; i++)
-	if (i != 7 && i != 10 && i != 13) printf "%c", i }' </dev/null`
-# Exercise backslashes adjacent to double quotes, a Windows special case.
-dbname1='\"\'$dbname1'\\"\\\'
-dbname2=`awk 'BEGIN { for (i = 46; i <  91; i++) printf "%c", i }' </dev/null`
-dbname3=`awk 'BEGIN { for (i = 91; i < 128; i++) printf "%c", i }' </dev/null`
-createdb "regression$dbname1" || createdb_status=$?
-createdb "regression$dbname2" || createdb_status=$?
-createdb "regression$dbname3" || createdb_status=$?
-
-# Extra options to apply to the dump.  This may be changed later.
-extra_dump_options=""
-
-if "$MAKE" -C "$oldsrc" installcheck-parallel; then
-	oldpgversion=`psql -X -A -t -d regression -c "SHOW server_version_num"`
-
-	# Before dumping, tweak the database of the old instance depending
-	# on its version.
-	if [ "$newsrc" != "$oldsrc" ]; then
-		# This SQL script has its own idea of the cleanup that needs to be
-		# done on the cluster to-be-upgraded, and includes version checks.
-		# Note that this uses the script stored on the new branch.
-		psql -X -d regression -f "$newsrc/src/bin/pg_upgrade/upgrade_adapt.sql" \
-			|| psql_fix_sql_status=$?
-
-		# Handling of --extra-float-digits gets messy after v12.
-		# Note that this changes the dumps from the old and new
-		# instances if involving an old cluster of v11 or older.
-		if [ $oldpgversion -lt 120000 ]; then
-			extra_dump_options="--extra-float-digits=0"
-		fi
-	fi
-
-	pg_dumpall $extra_dump_options --no-sync \
-		-f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
-
-	if [ "$newsrc" != "$oldsrc" ]; then
-		# update references to old source tree's regress.so etc
-		fix_sql=""
-		case $oldpgversion in
-			*)
-				fix_sql="UPDATE pg_proc SET probin = replace(probin, '$oldsrc', '$newsrc') WHERE probin LIKE '$oldsrc%';"
-				;;
-		esac
-		psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-
-		mv "$temp_root"/dump1.sql "$temp_root"/dump1.sql.orig
-		sed "s;$oldsrc;$newsrc;g" "$temp_root"/dump1.sql.orig >"$temp_root"/dump1.sql
-	fi
-else
-	make_installcheck_status=$?
-fi
-"$oldbindir"/pg_ctl -m fast stop
-if [ -n "$createdb_status" ]; then
-	exit 1
-fi
-if [ -n "$make_installcheck_status" ]; then
-	exit 1
-fi
-if [ -n "$psql_fix_sql_status" ]; then
-	exit 1
-fi
-if [ -n "$pg_dumpall1_status" ]; then
-	echo "pg_dumpall of pre-upgrade database cluster failed"
-	exit 1
-fi
-
-PGDATA="$BASE_PGDATA"
-
-standard_initdb 'initdb'
-
-pg_upgrade $PG_UPGRADE_OPTS --no-sync -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -p "$PGPORT" -P "$PGPORT"
-
-# make sure all directories and files have group permissions, on Unix hosts
-# Windows hosts don't support Unix-y permissions.
-case $testhost in
-	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type f ! -perm 640 | wc -l` -ne 0 ]; then
-			echo "files in PGDATA with permission != 640";
-			exit 1;
-		fi ;;
-esac
-
-case $testhost in
-	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type d ! -perm 750 | wc -l` -ne 0 ]; then
-			echo "directories in PGDATA with permission != 750";
-			exit 1;
-		fi ;;
-esac
-
-pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w
-
-pg_dumpall $extra_dump_options --no-sync \
-	-f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
-pg_ctl -m fast stop
-
-if [ -n "$pg_dumpall2_status" ]; then
-	echo "pg_dumpall of post-upgrade database cluster failed"
-	exit 1
-fi
-
-case $testhost in
-	MINGW*)	MSYS2_ARG_CONV_EXCL=/c cmd /c delete_old_cluster.bat ;;
-	*)	    sh ./delete_old_cluster.sh ;;
-esac
-
-if diff "$temp_root"/dump1.sql "$temp_root"/dump2.sql >/dev/null; then
-	echo PASSED
-	exit 0
-else
-	echo "Files $temp_root/dump1.sql and $temp_root/dump2.sql differ"
-	echo "dumps were not identical"
-	exit 1
-fi
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index e18f27276c..541b4b6b98 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -327,6 +327,31 @@ sub install_path
 
 =pod
 
+=item $node->config_data($option)
+
+Grab some data from pg_config, with $option being the command switch
+used.
+
+=cut
+
+sub config_data
+{
+	my ($self, $option) = @_;
+	local %ENV = $self->_get_env();
+
+	my ($stdout, $stderr);
+	my $result =
+	  IPC::Run::run [ $self->installed_command('pg_config'), $option ],
+	  '>', \$stdout, '2>', \$stderr
+	  or die "could not execute pg_config";
+	chomp($stdout);
+	$stdout =~ s/\r$//;
+
+	return $stdout;
+}
+
+=pod
+
 =item $node->info()
 
 Return a string containing human-readable diagnostic information (paths, etc)
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 7575acdfdf..b5c276fc7c 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -287,6 +287,10 @@ sub bincheck
 	foreach my $dir (@bin_dirs)
 	{
 		next unless -d "$dir/t";
+		# Do not consider pg_upgrade, as it is handled by
+		# upgradecheck.
+		next if ($dir =~ "/pg_upgrade/");
+
 		my $status = tap_check($dir);
 		$mstat ||= $status;
 	}
@@ -592,91 +596,11 @@ sub generate_db
 
 sub upgradecheck
 {
-	my $status;
-	my $cwd = getcwd();
-
-	# Much of this comes from the pg_upgrade test.sh script,
-	# but it only covers the --install case, and not the case
-	# where the old and new source or bin dirs are different.
-	# i.e. only this version to this version check. That's
-	# what pg_upgrade's "make check" does.
-
-	$ENV{PGHOST} = 'localhost';
-	$ENV{PGPORT} ||= 50432;
-	my $tmp_root = "$topdir/src/bin/pg_upgrade/tmp_check";
-	rmtree($tmp_root);
-	mkdir $tmp_root || die $!;
-	my $upg_tmp_install = "$tmp_root/install";    # unshared temp install
-	print "Setting up temp install\n\n";
-	Install($upg_tmp_install, "all", $config);
-
-	# Install does a chdir, so change back after that
-	chdir $cwd;
-	my ($bindir, $libdir, $oldsrc, $newsrc) =
-	  ("$upg_tmp_install/bin", "$upg_tmp_install/lib", $topdir, $topdir);
-	$ENV{PATH} = "$bindir;$ENV{PATH}";
-	my $data = "$tmp_root/data";
-	$ENV{PGDATA} = "$data.old";
-	my $outputdir          = "$tmp_root/regress";
-	my @EXTRA_REGRESS_OPTS = ("--outputdir=$outputdir");
-	mkdir "$outputdir" || die $!;
-
-	my $logdir = "$topdir/src/bin/pg_upgrade/log";
-	rmtree($logdir);
-	mkdir $logdir || die $!;
-	print "\nRunning initdb on old cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nStarting old cluster\n\n";
-	my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log");
-	system(@args) == 0 or exit 1;
-
-	print "\nCreating databases with names covering most ASCII bytes\n\n";
-	generate_db("\\\"\\", 1,  45,  "\\\\\"\\\\\\");
-	generate_db('',       46, 90,  '');
-	generate_db('',       91, 127, '');
-
-	print "\nSetting up data for upgrading\n\n";
-	installcheck_internal('parallel', @EXTRA_REGRESS_OPTS);
-
-	# now we can chdir into the source dir
-	chdir "$topdir/src/bin/pg_upgrade";
-	print "\nDumping old cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump1.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping old cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	$ENV{PGDATA} = "$data";
-	print "\nSetting up new cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nRunning pg_upgrade\n\n";
-	@args = (
-		'pg_upgrade', '-d', "$data.old", '-D', $data, '-b', $bindir,
-		'--no-sync');
-	system(@args) == 0 or exit 1;
-	print "\nStarting new cluster\n\n";
-	@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", 'start');
-	system(@args) == 0 or exit 1;
-	print "\nDumping new cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump2.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping new cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	print "\nDeleting old cluster\n\n";
-	system(".\\delete_old_cluster.bat") == 0 or exit 1;
-	print "\nComparing old and new cluster dumps\n\n";
-
-	@args = ('diff', '-q', "$tmp_root/dump1.sql", "$tmp_root/dump2.sql");
-	system(@args);
-	$status = $?;
-	if (!$status)
-	{
-		print "PASSED\n";
-	}
-	else
-	{
-		print "dumps not identical!\n";
-		exit(1);
-	}
+	InstallTemp();
+	# Tweak environment for the upgrade tests
+	$ENV{REGRESS_OUTPUTDIR} = "$topdir/src/bin/pg_upgrade";
+	my $mstat = tap_check("$topdir/src/bin/pg_upgrade");
+	exit $mstat if $mstat;
 	return;
 }
 
-- 
2.34.1



  [application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
@ 2022-01-15 05:52  Julien Rouhaud <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Julien Rouhaud @ 2022-01-15 05:52 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Andres Freund <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Postgres hackers <[email protected]>

Hi,

On Tue, Jan 11, 2022 at 04:14:25PM +0900, Michael Paquier wrote:
> The CF bot is unhappy, so here is a rebase, with some typo fixes
> reported by Justin offlist.

The cfbot still complains about this patch on Windows:
https://cirrus-ci.com/task/6411385683836928
https://api.cirrus-ci.com/v1/artifact/task/6411385683836928/tap/src/bin/pg_upgrade/tmp_check/log/reg...

# Running: pg_upgrade --no-sync -d c:/cirrus/src/bin/pg_upgrade/tmp_check/t_002_pg_upgrade_old_node_data/pgdata -D c:/cirrus/src/bin/pg_upgrade/tmp_check/t_002_pg_upgrade_new_node_data/pgdata -b C:/cirrus/tmp_install/bin -B C:/cirrus/tmp_install/bin -p 56296 -P 56297

libpq environment variable PGHOST has a non-local server value: C:/Users/ContainerAdministrator/AppData/Local/Temp/FhBIlsw6SV
Failure, exiting
not ok 3 - run of pg_upgrade for new instance

#   Failed test 'run of pg_upgrade for new instance'
#   at t/002_pg_upgrade.pl line 255.
### Starting node "new_node"
# Running: pg_ctl -w -D c:/cirrus/src/bin/pg_upgrade/tmp_check/t_002_pg_upgrade_new_node_data/pgdata -l c:/cirrus/src/bin/pg_upgrade/tmp_check/log/002_pg_upgrade_new_node.log -o --cluster-name=new_node start
waiting for server to start.... done
server started
# Postmaster PID for node "new_node" is 5748
# Running: pg_dumpall --no-sync -d port=56297 host=C:/Users/ContainerAdministrator/AppData/Local/Temp/FhBIlsw6SV dbname='postgres' -f C:\cirrus\src\bin\pg_upgrade\tmp_check\tmp_test_X4aZ/dump2.sql
# Running: diff -q C:\cirrus\src\bin\pg_upgrade\tmp_check\tmp_test_X4aZ/dump1.sql C:\cirrus\src\bin\pg_upgrade\tmp_check\tmp_test_X4aZ/dump2.sql
Files C:\cirrus\src\bin\pg_upgrade\tmp_check\tmp_test_X4aZ/dump1.sql and C:\cirrus\src\bin\pg_upgrade\tmp_check\tmp_test_X4aZ/dump2.sql differ
not ok 4 - old and new dump match after pg_upgrade

#   Failed test 'old and new dump match after pg_upgrade'

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
@ 2022-01-18 02:20  Michael Paquier <[email protected]>
  parent: Julien Rouhaud <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Michael Paquier @ 2022-01-18 02:20 UTC (permalink / raw)
  To: Julien Rouhaud <[email protected]>; +Cc: Andres Freund <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Postgres hackers <[email protected]>

On Sat, Jan 15, 2022 at 01:52:39PM +0800, Julien Rouhaud wrote:
> libpq environment variable PGHOST has a non-local server value: C:/Users/ContainerAdministrator/AppData/Local/Temp/FhBIlsw6SV
> Failure, exiting
> not ok 3 - run of pg_upgrade for new instance

There are two things here, as far as I understand:
1) This is a valid Windows path.  So shouldn't we fix pg_upgrade's
server.c to be a bit more compliant with Windows paths?  The code
accepts only paths beginning with '/' as local paths, so this breaks.
2) It looks safer in the long run to disable completely PGHOST and
PGHOSTADDR when running the pg_upgrade command in the test, and we'd
better not use Cluster::command_ok() or we would fall down to each
node's local environment.  This could be done in the tests as of the
attached, I guess, and this would bypass the problem coming from 1).

The patch needed a refresh as --make-testtablespace-dir has been
removed as of d6d317d.
--
Michael


Attachments:

  [text/x-diff] v7-0001-Switch-tests-of-pg_upgrade-to-use-TAP.patch (28.6K, ../../YeYj4DU5qY%[email protected]/2-v7-0001-Switch-tests-of-pg_upgrade-to-use-TAP.patch)
  download | inline diff:
From 5b4c3f279781418aa4bc24689342b41926d681e4 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Tue, 18 Jan 2022 11:13:09 +0900
Subject: [PATCH v7] Switch tests of pg_upgrade to use TAP

---
 src/bin/pg_upgrade/.gitignore            |   5 +
 src/bin/pg_upgrade/Makefile              |  23 +-
 src/bin/pg_upgrade/TESTING               |  33 ++-
 src/bin/pg_upgrade/t/001_basic.pl        |   9 +
 src/bin/pg_upgrade/t/002_pg_upgrade.pl   | 286 +++++++++++++++++++++++
 src/bin/pg_upgrade/test.sh               | 279 ----------------------
 src/test/perl/PostgreSQL/Test/Cluster.pm |  25 ++
 src/tools/msvc/vcregress.pl              |  94 +-------
 8 files changed, 366 insertions(+), 388 deletions(-)
 create mode 100644 src/bin/pg_upgrade/t/001_basic.pl
 create mode 100644 src/bin/pg_upgrade/t/002_pg_upgrade.pl
 delete mode 100644 src/bin/pg_upgrade/test.sh

diff --git a/src/bin/pg_upgrade/.gitignore b/src/bin/pg_upgrade/.gitignore
index 2d3bfeaa50..3b64522ab6 100644
--- a/src/bin/pg_upgrade/.gitignore
+++ b/src/bin/pg_upgrade/.gitignore
@@ -7,3 +7,8 @@
 /loadable_libraries.txt
 /log/
 /tmp_check/
+
+# Generated by pg_regress
+/sql/
+/expected/
+/results/
diff --git a/src/bin/pg_upgrade/Makefile b/src/bin/pg_upgrade/Makefile
index 44d06be5a6..35b6c123a5 100644
--- a/src/bin/pg_upgrade/Makefile
+++ b/src/bin/pg_upgrade/Makefile
@@ -28,6 +28,12 @@ OBJS = \
 override CPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS)
 LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
 
+# required for 002_pg_upgrade.pl
+REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
+export REGRESS_SHLIB
+REGRESS_OUTPUTDIR=$(abs_top_builddir)/src/bin/pg_upgrade
+export REGRESS_OUTPUTDIR
+
 all: pg_upgrade
 
 pg_upgrade: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
@@ -49,17 +55,8 @@ clean distclean maintainer-clean:
 	       pg_upgrade_dump_globals.sql \
 	       pg_upgrade_dump_*.custom pg_upgrade_*.log
 
-# When $(MAKE) is present, make automatically infers that this is a
-# recursive make. which is not actually what we want here, as that
-# e.g. prevents output synchronization from working (as make thinks
-# that the subsidiary make knows how to deal with that itself, but
-# we're invoking a shell script that doesn't know). Referencing
-# $(MAKE) indirectly avoids that behaviour.
-# See https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html#MAKE-Variable
-NOTSUBMAKEMAKE=$(MAKE)
+check:
+	$(prove_check)
 
-check: test.sh all temp-install
-	MAKE=$(NOTSUBMAKEMAKE) $(with_temp_install) bindir=$(abs_top_builddir)/tmp_install/$(bindir) EXTRA_REGRESS_OPTS="$(EXTRA_REGRESS_OPTS)" $(SHELL) $<
-
-# installcheck is not supported because there's no meaningful way to test
-# pg_upgrade against a single already-running server
+installcheck:
+	$(prove_installcheck)
diff --git a/src/bin/pg_upgrade/TESTING b/src/bin/pg_upgrade/TESTING
index 78b9747908..43a71566e2 100644
--- a/src/bin/pg_upgrade/TESTING
+++ b/src/bin/pg_upgrade/TESTING
@@ -2,21 +2,30 @@ THE SHORT VERSION
 -----------------
 
 On non-Windows machines, you can execute the testing process
-described below by running
+described below by running the following command in this directory:
 	make check
-in this directory.  This will run the shell script test.sh, performing
-an upgrade from the version in this source tree to a new instance of
-the same version.
 
-To test an upgrade from a different version, you must have a built
-source tree for the old version as well as this version, and you
-must have done "make install" for both versions.  Then do:
+This will run the TAP tests to run pg_upgrade, performing an upgrade
+from the version in this source tree to a  new instance of the same
+version.
+
+To test an upgrade from a different version, there are two options
+available:
+
+1) You have a built source tree for the old version as well as this
+version's binaries.  Then set up the following variables before
+launching the test:
 
 export oldsrc=...somewhere/postgresql	(old version's source tree)
-export oldbindir=...otherversion/bin	(old version's installed bin dir)
-export bindir=...thisversion/bin	(this version's installed bin dir)
-export libdir=...thisversion/lib	(this version's installed lib dir)
-sh test.sh
+export oldinstall=...otherversion/	(old version's install base path)
+
+2) You have a dump that can be used to set up the old version, as well
+as this version's binaries.  Then set up the following variables:
+export olddump=...somewhere/dump.sql	(old version's dump)
+export oldinstall=...otherversion/	(old version's install base path)
+
+Finally, the tests can be done by running
+	make check
 
 In this case, you will have to manually eyeball the resulting dump
 diff for version-specific differences, as explained below.
@@ -77,3 +86,5 @@ steps:
 
 7)  Diff the regression database dump file with the regression dump
     file loaded into the old server.
+
+The generated dump may be reusable with "olddump", as defined above.
diff --git a/src/bin/pg_upgrade/t/001_basic.pl b/src/bin/pg_upgrade/t/001_basic.pl
new file mode 100644
index 0000000000..75b0f98b08
--- /dev/null
+++ b/src/bin/pg_upgrade/t/001_basic.pl
@@ -0,0 +1,9 @@
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Utils;
+use Test::More tests => 8;
+
+program_help_ok('pg_upgrade');
+program_version_ok('pg_upgrade');
+program_options_handling_ok('pg_upgrade');
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
new file mode 100644
index 0000000000..fa8933095a
--- /dev/null
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -0,0 +1,286 @@
+# Set of tests for pg_upgrade, including cross-version checks.
+use strict;
+use warnings;
+
+use Cwd qw(abs_path getcwd);
+use File::Basename qw(dirname);
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 4;
+
+# Generate a database with a name made of a range of ASCII characters.
+sub generate_db
+{
+	my ($node, $from_char, $to_char) = @_;
+
+	my $dbname = '';
+	for my $i ($from_char .. $to_char)
+	{
+		next if $i == 7 || $i == 10 || $i == 13;    # skip BEL, LF, and CR
+		$dbname = $dbname . sprintf('%c', $i);
+	}
+	$node->run_log([ 'createdb', '--port', $node->port, $dbname ]);
+}
+
+# From now on, the test of pg_upgrade consists in setting up an instance.
+# This is the source instance used for the upgrade. Then a new and fresh
+# instance is created, and is used as the target instance for the
+# upgrade.  Before running an upgrade, a logical dump of the old instance
+# is taken, and a second logical dump of the new instance is taken after
+# the upgrade.  The upgrade test passes if there are no differences after
+# running pg_upgrade.
+
+# Testing upgrades with an older instance of PostgreSQL requires
+# setting up two environment variables, among the following:
+# - "oldsrc", to point to the code source of the older version.
+#   This is required to set up the old instance with pg_upgrade.
+# - "olddump", to point to a dump file that will be used to set
+#   up the old instance to upgrade from.
+# - "oldinstall", to point to the installation path of the older
+# version.
+
+# "oldsrc" and "olddump" cannot be used together.  Setting up
+# "olddump" and "oldinstall" will use the dump pointed to to
+# set up the old instance.  If "oldsrc" is used instead of "olddump",
+# the full set of regression tests of the old instance is run
+# instead.
+
+if (defined($ENV{oldsrc}) && defined($ENV{olddump}))
+{
+	die "oldsrc and olddump are both defined";
+}
+elsif (defined($ENV{oldsrc}))
+{
+	if (   (defined($ENV{oldsrc}) && !defined($ENV{oldinstall}))
+		|| (!defined($ENV{oldsrc}) && defined($ENV{oldinstall})))
+	{
+		# Not all variables are defined, so leave and die if test is
+		# done with an older installation.
+		die "oldsrc or oldinstall is undefined";
+	}
+}
+elsif (defined($ENV{olddump}))
+{
+	if (   (defined($ENV{olddump}) && !defined($ENV{oldinstall}))
+		|| (!defined($ENV{olddump}) && defined($ENV{oldinstall})))
+	{
+		# Not all variables are defined, so leave and die if test is
+		# done with an older installation.
+		die "olddump or oldinstall is undefined";
+	}
+}
+
+if ((defined($ENV{oldsrc}) || defined($ENV{olddump})) && $windows_os)
+{
+	# This configuration is not supported on Windows, as regress.so
+	# location diverges across the compilation methods used on this
+	# platform.
+	die "No support for older version tests on Windows";
+}
+
+# Default is the location of this source code for both nodes used with
+# the upgrade.
+my $newsrc = abs_path("../../..");
+my $oldsrc = $ENV{oldsrc} || $newsrc;
+$oldsrc = abs_path($oldsrc);
+
+# Temporary location for the dumps taken
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+
+# Initialize node to upgrade
+my $oldnode = PostgreSQL::Test::Cluster->new('old_node',
+	install_path => $ENV{oldinstall});
+
+$oldnode->init(extra => [ '--locale', 'C', '--encoding', 'LATIN1' ]);
+$oldnode->start;
+
+# Set up the data of the old instance with pg_regress or an old dump.
+if (defined($ENV{olddump}))
+{
+	# Use the dump specified.
+	my $olddumpfile = $ENV{olddump};
+	die "no dump file found!" unless -e $olddumpfile;
+
+	# Load the dump, and we are done here.
+	$oldnode->command_ok(
+		[
+			'psql',   '-X',           '-f', $olddumpfile,
+			'--port', $oldnode->port, 'regression'
+		]);
+}
+else
+{
+	# Default is to just use pg_regress to set up the old instance
+	# Creating databases with names covering most ASCII bytes
+	generate_db($oldnode, 1,  45);
+	generate_db($oldnode, 46, 90);
+	generate_db($oldnode, 91, 127);
+
+	# Run core regression tests on the old instance.
+	$oldnode->run_log([ "createdb", '--port', $oldnode->port, 'regression' ]);
+
+	# Grab any regression options that may be passed down by caller.
+	my $extra_opts_val = $ENV{EXTRA_REGRESS_OPT} || "";
+	my @extra_opts     = split(/\s+/, $extra_opts_val);
+
+	# --dlpath is needed to be able to find the location of regress.so and
+	# any libraries the regression tests required.  This needs to point to
+	# the old instance when using it.  In the default case, fallback to
+	# what the caller provided for REGRESS_SHLIB.
+	my $dlpath;
+	if (defined($ENV{oldinstall}))
+	{
+		$dlpath = "$oldsrc/src/test/regress";
+	}
+	else
+	{
+		$dlpath = dirname($ENV{REGRESS_SHLIB});
+	}
+	$dlpath = PostgreSQL::Test::Utils::perl2host($dlpath);
+
+	# --outputdir points to the path where to place the output files, which
+	# had better be this directory.
+	my $outputdir =
+	  PostgreSQL::Test::Utils::perl2host($ENV{REGRESS_OUTPUTDIR});
+
+	# --inputdir needs to point to the location of the input files, from the
+	# cluster to-be-upgraded.
+	my $inputdir =
+	  PostgreSQL::Test::Utils::perl2host("$oldsrc/src/test/regress");
+
+	my @regress_command = [
+		$ENV{PG_REGRESS},
+		'--schedule',     "$oldsrc/src/test/regress/parallel_schedule",
+		'--bindir',       $oldnode->config_data('--bindir'),
+		'--dlpath',       $dlpath,
+		'--port',         $oldnode->port,
+		'--outputdir',    $outputdir,
+		'--inputdir',     $inputdir,
+		'--use-existing'
+	];
+	@regress_command = (@regress_command, @extra_opts);
+
+	$oldnode->command_ok(@regress_command,
+		'regression test run on old instance');
+}
+
+# Before dumping, get rid of objects not existing or not supported in later
+# versions. This depends on the version of the old server used, and matters
+# only if different versions are used for the dump.
+if (defined($ENV{oldinstall}))
+{
+	# Note that upgrade_adapt.sql from the new version is used, to
+	# cope with an upgrade to this version.
+	$oldnode->run_log(
+		[
+			'psql', '-X', '-f',
+			PostgreSQL::Test::Utils::perl2host(
+				"$newsrc/src/bin/pg_upgrade/upgrade_adapt.sql"),
+			'--port',
+			$oldnode->port,
+			'regression'
+		]);
+}
+
+# Initialize a new node for the upgrade.  This is done early so as it is
+# possible to know with which node's PATH the initial dump needs to be
+# taken.
+my $newnode = PostgreSQL::Test::Cluster->new('new_node');
+$newnode->init(extra => [ '--locale=C', '--encoding=LATIN1' ]);
+my $newbindir = $newnode->config_data('--bindir');
+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');
+
+# After dumping, update references to the old source tree's regress.so
+# to point to the new tree.
+if (defined($ENV{oldinstall}))
+{
+	# First, fetch all the references to libraries that are not part
+	# of the default path $libdir.
+	my $output = $oldnode->safe_psql('regression',
+		"SELECT DISTINCT probin::text FROM pg_proc WHERE probin NOT LIKE '\$libdir%';"
+	);
+	chomp($output);
+	my @libpaths = split("\n", $output);
+
+	my $dump_data = slurp_file("$tempdir/dump1.sql");
+
+	my $newregresssrc = "$newsrc/src/test/regress";
+	foreach (@libpaths)
+	{
+		my $libpath = $_;
+		$libpath = dirname($libpath);
+		$dump_data =~ s/$libpath/$newregresssrc/g;
+	}
+
+	open my $fh, ">", "$tempdir/dump1.sql" or die "could not open dump file";
+	print $fh $dump_data;
+	close $fh;
+
+	# This replaces any references to the old tree's regress.so
+	# the new tree's regress.so.  Any references that do *not*
+	# match $libdir are switched so as this request does not
+	# depend on the path of the old source tree.  This is useful
+	# when using an old dump.  Do the operation on all the databases
+	# that allow connections so as this includes the regression
+	# database and anything the user has set up.
+	$output = $oldnode->safe_psql('postgres',
+		"SELECT datname FROM pg_database WHERE datallowconn;");
+	chomp($output);
+	my @datnames = split("\n", $output);
+	foreach (@datnames)
+	{
+		my $datname = $_;
+		$oldnode->safe_psql(
+			$datname, "UPDATE pg_proc SET probin =
+		  regexp_replace(probin, '.*/', '$newregresssrc/')
+		  WHERE probin NOT LIKE '\$libdir/%'");
+	}
+}
+
+# Update the instance.
+$oldnode->stop;
+
+# Time for the real run.
+# pg_upgrade would complain if PGHOST, so as there are no attempts to
+# connect to a different server than the upgraded ones.  So reset it
+# temporarily.  For the same reason, we should not rely on
+# PostgreSQL::Test::Cluster::command_ok() as this would load a node's
+# loal environment variables.
+my $pghost_local = $ENV{PGHOST};
+my $pghostaddr_local = $ENV{PGHOSTADDR};
+delete $ENV{PGHOST};
+delete $ENV{PGHOSTADDR};
+command_ok(
+	[
+		'pg_upgrade', '--no-sync',        '-d', $oldnode->data_dir,
+		'-D',         $newnode->data_dir, '-b', $oldbindir,
+		'-B',         $newbindir,         '-p', $oldnode->port,
+		'-P',         $newnode->port,
+	],
+	'run of pg_upgrade for new instance');
+$ENV{PGHOST} = $pghost_local;
+$ENV{PGHOSTADDR} = $pghostaddr_local;
+$newnode->start;
+
+# Take a second dump on the upgraded instance.
+$newnode->run_log(
+	[
+		'pg_dumpall', '--no-sync',
+		'-d',         $newnode->connstr('postgres'),
+		'-f',         "$tempdir/dump2.sql"
+	]);
+
+# Compare the two dumps, there should be no differences.
+command_ok([ 'diff', '-q', "$tempdir/dump1.sql", "$tempdir/dump2.sql" ],
+	'old and new dump match after pg_upgrade');
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
deleted file mode 100644
index ef328b3062..0000000000
--- a/src/bin/pg_upgrade/test.sh
+++ /dev/null
@@ -1,279 +0,0 @@
-#!/bin/sh
-
-# src/bin/pg_upgrade/test.sh
-#
-# Test driver for pg_upgrade.  Initializes a new database cluster,
-# runs the regression tests (to put in some data), runs pg_dumpall,
-# runs pg_upgrade, runs pg_dumpall again, compares the dumps.
-#
-# Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
-# Portions Copyright (c) 1994, Regents of the University of California
-
-set -e
-
-: ${MAKE=make}
-
-# Guard against parallel make issues (see comments in pg_regress.c)
-unset MAKEFLAGS
-unset MAKELEVEL
-
-# Run a given "initdb" binary and overlay the regression testing
-# authentication configuration.
-standard_initdb() {
-	# To increase coverage of non-standard segment size and group access
-	# without increasing test runtime, run these tests with a custom setting.
-	# Also, specify "-A trust" explicitly to suppress initdb's warning.
-	# --allow-group-access and --wal-segsize have been added in v11.
-	"$1" -N --wal-segsize 1 --allow-group-access -A trust
-	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
-	then
-		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
-	fi
-	../../test/regress/pg_regress --config-auth "$PGDATA"
-}
-
-# What flavor of host are we on?
-# Treat MINGW* (msys1) and MSYS* (msys2) the same.
-testhost=`uname -s | sed 's/^MSYS/MINGW/'`
-
-# Establish how the server will listen for connections
-case $testhost in
-	MINGW*)
-		LISTEN_ADDRESSES="localhost"
-		PG_REGRESS_SOCKET_DIR=""
-		PGHOST=localhost
-		;;
-	*)
-		LISTEN_ADDRESSES=""
-		# Select a socket directory.  The algorithm is from the "configure"
-		# script; the outcome mimics pg_regress.c:make_temp_sockdir().
-		if [ x"$PG_REGRESS_SOCKET_DIR" = x ]; then
-			set +e
-			dir=`(umask 077 &&
-				  mktemp -d /tmp/pg_upgrade_check-XXXXXX) 2>/dev/null`
-			if [ ! -d "$dir" ]; then
-				dir=/tmp/pg_upgrade_check-$$-$RANDOM
-				(umask 077 && mkdir "$dir")
-				if [ ! -d "$dir" ]; then
-					echo "could not create socket temporary directory in \"/tmp\""
-					exit 1
-				fi
-			fi
-			set -e
-			PG_REGRESS_SOCKET_DIR=$dir
-			trap 'rm -rf "$PG_REGRESS_SOCKET_DIR"' 0
-			trap 'exit 3' 1 2 13 15
-		fi
-		PGHOST=$PG_REGRESS_SOCKET_DIR
-		;;
-esac
-
-POSTMASTER_OPTS="-F -c listen_addresses=\"$LISTEN_ADDRESSES\" -k \"$PG_REGRESS_SOCKET_DIR\""
-export PGHOST
-
-# don't rely on $PWD here, as old shells don't set it
-temp_root=`pwd`/tmp_check
-rm -rf "$temp_root"
-mkdir "$temp_root"
-
-: ${oldbindir=$bindir}
-
-: ${oldsrc=../../..}
-oldsrc=`cd "$oldsrc" && pwd`
-newsrc=`cd ../../.. && pwd`
-
-# We need to make pg_regress use psql from the desired installation
-# (likely a temporary one), because otherwise the installcheck run
-# below would try to use psql from the proper installation directory
-# of the target version, which might be outdated or not exist. But
-# don't override anything else that's already in EXTRA_REGRESS_OPTS.
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --bindir='$oldbindir'"
-export EXTRA_REGRESS_OPTS
-
-# While in normal cases this will already be set up, adding bindir to
-# path allows test.sh to be invoked with different versions as
-# described in ./TESTING
-PATH=$bindir:$PATH
-export PATH
-
-BASE_PGDATA="$temp_root/data"
-PGDATA="${BASE_PGDATA}.old"
-export PGDATA
-
-# Send installcheck outputs to a private directory.  This avoids conflict when
-# check-world runs pg_upgrade check concurrently with src/test/regress check.
-# To retrieve interesting files after a run, use pattern tmp_check/*/*.diffs.
-outputdir="$temp_root/regress"
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"
-export EXTRA_REGRESS_OPTS
-mkdir "$outputdir"
-
-# pg_regress --make-tablespacedir would take care of that in 14~, but this is
-# still required for older versions where this option is not supported.
-if [ "$newsrc" != "$oldsrc" ]; then
-	mkdir "$outputdir"/testtablespace
-	mkdir "$outputdir"/sql
-	mkdir "$outputdir"/expected
-fi
-
-logdir=`pwd`/log
-rm -rf "$logdir"
-mkdir "$logdir"
-
-# Clear out any environment vars that might cause libpq to connect to
-# the wrong postmaster (cf pg_regress.c)
-#
-# Some shells, such as NetBSD's, return non-zero from unset if the variable
-# is already unset. Since we are operating under 'set -e', this causes the
-# script to fail. To guard against this, set them all to an empty string first.
-PGDATABASE="";        unset PGDATABASE
-PGUSER="";            unset PGUSER
-PGSERVICE="";         unset PGSERVICE
-PGSSLMODE="";         unset PGSSLMODE
-PGREQUIRESSL="";      unset PGREQUIRESSL
-PGCONNECT_TIMEOUT=""; unset PGCONNECT_TIMEOUT
-PGHOSTADDR="";        unset PGHOSTADDR
-
-# Select a non-conflicting port number, similarly to pg_regress.c
-PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$newsrc"/src/include/pg_config.h | awk '{print $3}'`
-PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
-export PGPORT
-
-i=0
-while psql -X postgres </dev/null 2>/dev/null
-do
-	i=`expr $i + 1`
-	if [ $i -eq 16 ]
-	then
-		echo port $PGPORT apparently in use
-		exit 1
-	fi
-	PGPORT=`expr $PGPORT + 1`
-	export PGPORT
-done
-
-# buildfarm may try to override port via EXTRA_REGRESS_OPTS ...
-EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --port=$PGPORT"
-export EXTRA_REGRESS_OPTS
-
-standard_initdb "$oldbindir"/initdb
-"$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
-
-# Create databases with names covering the ASCII bytes other than NUL, BEL,
-# LF, or CR.  BEL would ring the terminal bell in the course of this test, and
-# it is not otherwise a special case.  PostgreSQL doesn't support the rest.
-dbname1=`awk 'BEGIN { for (i= 1; i < 46; i++)
-	if (i != 7 && i != 10 && i != 13) printf "%c", i }' </dev/null`
-# Exercise backslashes adjacent to double quotes, a Windows special case.
-dbname1='\"\'$dbname1'\\"\\\'
-dbname2=`awk 'BEGIN { for (i = 46; i <  91; i++) printf "%c", i }' </dev/null`
-dbname3=`awk 'BEGIN { for (i = 91; i < 128; i++) printf "%c", i }' </dev/null`
-createdb "regression$dbname1" || createdb_status=$?
-createdb "regression$dbname2" || createdb_status=$?
-createdb "regression$dbname3" || createdb_status=$?
-
-# Extra options to apply to the dump.  This may be changed later.
-extra_dump_options=""
-
-if "$MAKE" -C "$oldsrc" installcheck-parallel; then
-	oldpgversion=`psql -X -A -t -d regression -c "SHOW server_version_num"`
-
-	# Before dumping, tweak the database of the old instance depending
-	# on its version.
-	if [ "$newsrc" != "$oldsrc" ]; then
-		# This SQL script has its own idea of the cleanup that needs to be
-		# done on the cluster to-be-upgraded, and includes version checks.
-		# Note that this uses the script stored on the new branch.
-		psql -X -d regression -f "$newsrc/src/bin/pg_upgrade/upgrade_adapt.sql" \
-			|| psql_fix_sql_status=$?
-
-		# Handling of --extra-float-digits gets messy after v12.
-		# Note that this changes the dumps from the old and new
-		# instances if involving an old cluster of v11 or older.
-		if [ $oldpgversion -lt 120000 ]; then
-			extra_dump_options="--extra-float-digits=0"
-		fi
-	fi
-
-	pg_dumpall $extra_dump_options --no-sync \
-		-f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
-
-	if [ "$newsrc" != "$oldsrc" ]; then
-		# update references to old source tree's regress.so etc
-		fix_sql=""
-		case $oldpgversion in
-			*)
-				fix_sql="UPDATE pg_proc SET probin = replace(probin, '$oldsrc', '$newsrc') WHERE probin LIKE '$oldsrc%';"
-				;;
-		esac
-		psql -X -d regression -c "$fix_sql;" || psql_fix_sql_status=$?
-
-		mv "$temp_root"/dump1.sql "$temp_root"/dump1.sql.orig
-		sed "s;$oldsrc;$newsrc;g" "$temp_root"/dump1.sql.orig >"$temp_root"/dump1.sql
-	fi
-else
-	make_installcheck_status=$?
-fi
-"$oldbindir"/pg_ctl -m fast stop
-if [ -n "$createdb_status" ]; then
-	exit 1
-fi
-if [ -n "$make_installcheck_status" ]; then
-	exit 1
-fi
-if [ -n "$psql_fix_sql_status" ]; then
-	exit 1
-fi
-if [ -n "$pg_dumpall1_status" ]; then
-	echo "pg_dumpall of pre-upgrade database cluster failed"
-	exit 1
-fi
-
-PGDATA="$BASE_PGDATA"
-
-standard_initdb 'initdb'
-
-pg_upgrade $PG_UPGRADE_OPTS --no-sync -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -p "$PGPORT" -P "$PGPORT"
-
-# make sure all directories and files have group permissions, on Unix hosts
-# Windows hosts don't support Unix-y permissions.
-case $testhost in
-	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type f ! -perm 640 | wc -l` -ne 0 ]; then
-			echo "files in PGDATA with permission != 640";
-			exit 1;
-		fi ;;
-esac
-
-case $testhost in
-	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type d ! -perm 750 | wc -l` -ne 0 ]; then
-			echo "directories in PGDATA with permission != 750";
-			exit 1;
-		fi ;;
-esac
-
-pg_ctl start -l "$logdir/postmaster2.log" -o "$POSTMASTER_OPTS" -w
-
-pg_dumpall $extra_dump_options --no-sync \
-	-f "$temp_root"/dump2.sql || pg_dumpall2_status=$?
-pg_ctl -m fast stop
-
-if [ -n "$pg_dumpall2_status" ]; then
-	echo "pg_dumpall of post-upgrade database cluster failed"
-	exit 1
-fi
-
-case $testhost in
-	MINGW*)	MSYS2_ARG_CONV_EXCL=/c cmd /c delete_old_cluster.bat ;;
-	*)	    sh ./delete_old_cluster.sh ;;
-esac
-
-if diff "$temp_root"/dump1.sql "$temp_root"/dump2.sql >/dev/null; then
-	echo PASSED
-	exit 0
-else
-	echo "Files $temp_root/dump1.sql and $temp_root/dump2.sql differ"
-	echo "dumps were not identical"
-	exit 1
-fi
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 7af0f8db13..7fb412d6a5 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -327,6 +327,31 @@ sub install_path
 
 =pod
 
+=item $node->config_data($option)
+
+Grab some data from pg_config, with $option being the command switch
+used.
+
+=cut
+
+sub config_data
+{
+	my ($self, $option) = @_;
+	local %ENV = $self->_get_env();
+
+	my ($stdout, $stderr);
+	my $result =
+	  IPC::Run::run [ $self->installed_command('pg_config'), $option ],
+	  '>', \$stdout, '2>', \$stderr
+	  or die "could not execute pg_config";
+	chomp($stdout);
+	$stdout =~ s/\r$//;
+
+	return $stdout;
+}
+
+=pod
+
 =item $node->info()
 
 Return a string containing human-readable diagnostic information (paths, etc)
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 6dcd742fa6..e0797dc714 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -285,6 +285,10 @@ sub bincheck
 	foreach my $dir (@bin_dirs)
 	{
 		next unless -d "$dir/t";
+		# Do not consider pg_upgrade, as it is handled by
+		# upgradecheck.
+		next if ($dir =~ "/pg_upgrade/");
+
 		my $status = tap_check($dir);
 		$mstat ||= $status;
 	}
@@ -592,91 +596,11 @@ sub generate_db
 
 sub upgradecheck
 {
-	my $status;
-	my $cwd = getcwd();
-
-	# Much of this comes from the pg_upgrade test.sh script,
-	# but it only covers the --install case, and not the case
-	# where the old and new source or bin dirs are different.
-	# i.e. only this version to this version check. That's
-	# what pg_upgrade's "make check" does.
-
-	$ENV{PGHOST} = 'localhost';
-	$ENV{PGPORT} ||= 50432;
-	my $tmp_root = "$topdir/src/bin/pg_upgrade/tmp_check";
-	rmtree($tmp_root);
-	mkdir $tmp_root || die $!;
-	my $upg_tmp_install = "$tmp_root/install";    # unshared temp install
-	print "Setting up temp install\n\n";
-	Install($upg_tmp_install, "all", $config);
-
-	# Install does a chdir, so change back after that
-	chdir $cwd;
-	my ($bindir, $libdir, $oldsrc, $newsrc) =
-	  ("$upg_tmp_install/bin", "$upg_tmp_install/lib", $topdir, $topdir);
-	$ENV{PATH} = "$bindir;$ENV{PATH}";
-	my $data = "$tmp_root/data";
-	$ENV{PGDATA} = "$data.old";
-	my $outputdir          = "$tmp_root/regress";
-	my @EXTRA_REGRESS_OPTS = ("--outputdir=$outputdir");
-	mkdir "$outputdir" || die $!;
-
-	my $logdir = "$topdir/src/bin/pg_upgrade/log";
-	rmtree($logdir);
-	mkdir $logdir || die $!;
-	print "\nRunning initdb on old cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nStarting old cluster\n\n";
-	my @args = ('pg_ctl', 'start', '-l', "$logdir/postmaster1.log");
-	system(@args) == 0 or exit 1;
-
-	print "\nCreating databases with names covering most ASCII bytes\n\n";
-	generate_db("\\\"\\", 1,  45,  "\\\\\"\\\\\\");
-	generate_db('',       46, 90,  '');
-	generate_db('',       91, 127, '');
-
-	print "\nSetting up data for upgrading\n\n";
-	installcheck_internal('parallel', @EXTRA_REGRESS_OPTS);
-
-	# now we can chdir into the source dir
-	chdir "$topdir/src/bin/pg_upgrade";
-	print "\nDumping old cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump1.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping old cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	$ENV{PGDATA} = "$data";
-	print "\nSetting up new cluster\n\n";
-	standard_initdb() or exit 1;
-	print "\nRunning pg_upgrade\n\n";
-	@args = (
-		'pg_upgrade', '-d', "$data.old", '-D', $data, '-b', $bindir,
-		'--no-sync');
-	system(@args) == 0 or exit 1;
-	print "\nStarting new cluster\n\n";
-	@args = ('pg_ctl', '-l', "$logdir/postmaster2.log", 'start');
-	system(@args) == 0 or exit 1;
-	print "\nDumping new cluster\n\n";
-	@args = ('pg_dumpall', '-f', "$tmp_root/dump2.sql");
-	system(@args) == 0 or exit 1;
-	print "\nStopping new cluster\n\n";
-	system("pg_ctl stop") == 0 or exit 1;
-	print "\nDeleting old cluster\n\n";
-	system(".\\delete_old_cluster.bat") == 0 or exit 1;
-	print "\nComparing old and new cluster dumps\n\n";
-
-	@args = ('diff', '-q', "$tmp_root/dump1.sql", "$tmp_root/dump2.sql");
-	system(@args);
-	$status = $?;
-	if (!$status)
-	{
-		print "PASSED\n";
-	}
-	else
-	{
-		print "dumps not identical!\n";
-		exit(1);
-	}
+	InstallTemp();
+	# Tweak environment for the upgrade tests
+	$ENV{REGRESS_OUTPUTDIR} = "$topdir/src/bin/pg_upgrade";
+	my $mstat = tap_check("$topdir/src/bin/pg_upgrade");
+	exit $mstat if $mstat;
 	return;
 }
 
-- 
2.34.1



  [application/pgp-signature] signature.asc (833B, ../../YeYj4DU5qY%[email protected]/3-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
@ 2022-02-13 04:12  Andres Freund <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: Andres Freund @ 2022-02-13 04:12 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; +Cc: Julien Rouhaud <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Postgres hackers <[email protected]>

Hi,

On 2022-01-18 11:20:16 +0900, Michael Paquier wrote:
> On Sat, Jan 15, 2022 at 01:52:39PM +0800, Julien Rouhaud wrote:
> > libpq environment variable PGHOST has a non-local server value: C:/Users/ContainerAdministrator/AppData/Local/Temp/FhBIlsw6SV
> > Failure, exiting
> > not ok 3 - run of pg_upgrade for new instance
> 
> There are two things here, as far as I understand:
> 1) This is a valid Windows path.  So shouldn't we fix pg_upgrade's
> server.c to be a bit more compliant with Windows paths?  The code
> accepts only paths beginning with '/' as local paths, so this breaks.

It also doesn't handle @ correctly. Makes sense to fix. Should probably use
the same logic that libpq, psql, ... use?

			if (is_unixsock_path(ch->host))
				ch->type = CHT_UNIX_SOCKET;

that'd basically be the same amount of code. And easier to understand.

Greetings,

Andres Freund






^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set
@ 2022-02-14 08:01  Michael Paquier <[email protected]>
  parent: Andres Freund <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Michael Paquier @ 2022-02-14 08:01 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Julien Rouhaud <[email protected]>; Tom Lane <[email protected]>; Andrew Dunstan <[email protected]>; Postgres hackers <[email protected]>

On Sat, Feb 12, 2022 at 08:12:42PM -0800, Andres Freund wrote:
> It also doesn't handle @ correctly. Makes sense to fix. Should probably use
> the same logic that libpq, psql, ... use?
> 
> 			if (is_unixsock_path(ch->host))
> 				ch->type = CHT_UNIX_SOCKET;
> 
> that'd basically be the same amount of code. And easier to understand.

So, I am catching up with some parts of this thread, and I have
managed to miss is_unixsock_path().  Except if I am missing something
(now it is close to the end of the day here), a minimal change would
be something like that as we'd still want to allow the use of
localhost and others:
    if (value && strlen(value) > 0 &&
    /* check for 'local' host values */
        (strcmp(value, "localhost") != 0 && strcmp(value, "127.0.0.1") != 0 &&
-        strcmp(value, "::1") != 0 && value[0] != '/'))
+        strcmp(value, "::1") != 0 && !is_unixsock_path(value)))

Or perhaps we should restrict more the use of localhost values for
non-WIN32?  Opinions?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../YgoMQQgZrQihu%[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 7+ messages in thread

end of thread, other threads:[~2022-02-14 08:01 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 03:11 [PATCH v8 4/8] Propagate changes to indisclustered to child/parents Justin Pryzby <[email protected]>
2022-01-05 08:12 Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set Michael Paquier <[email protected]>
2022-01-11 07:14 ` Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set Michael Paquier <[email protected]>
2022-01-15 05:52   ` Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set Julien Rouhaud <[email protected]>
2022-01-18 02:20     ` Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set Michael Paquier <[email protected]>
2022-02-13 04:12       ` Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set Andres Freund <[email protected]>
2022-02-14 08:01         ` Re: Rewriting the test of pg_upgrade as a TAP test - take three - remastered set Michael Paquier <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox