On Wed, Oct 6, 2021 at 7:10 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
> I wonder if for Windows we'd want to switch to real symlinks, since,
> as far as I know from some light reading, reparse points are converted
> to absolute paths on creation, so the pgdata directory would be less
> portable than it would be on POSIX systems.
I looked into that a bit, and it seems that the newer "real" symlinks
can't be created without admin privileges, unlike junction points, so
that wouldn't help.  I still need to figure out what this
junction-based patch set is doing wrong on Windows though trial-by-CI.
In the meantime, here is a rebase over recent changes to the Perl
testing APIs.
From 0a76e6570b8f6eeac146a726a82c7e0e2315cdff Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Tue, 25 May 2021 20:42:17 +1200
Subject: [PATCH v6 1/4] Allow restricted relative tablespace paths.

Traditionally, tablespace paths provided to LOCATION had to be absolute,
because relative paths risked colliding with PostgreSQL-managed files
and generally blurring the boundary between system-managed and
user-managed data.

This commit adds a very small exception: it allows relative paths to be
used, but only under a new directory "pg_user_files".  This is intended
to allow automated testing of replication in later patches.

Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
 src/backend/commands/tablespace.c | 23 ++++++++++++++++++++---
 src/bin/initdb/initdb.c           |  1 +
 src/common/string.c               |  9 +++++++++
 src/include/common/string.h       |  1 +
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 4b96eec9df..adfbde6b29 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -70,6 +70,7 @@
 #include "commands/tablecmds.h"
 #include "commands/tablespace.h"
 #include "common/file_perm.h"
+#include "common/string.h"
 #include "miscadmin.h"
 #include "postmaster/bgwriter.h"
 #include "storage/fd.h"
@@ -267,14 +268,16 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 				 errmsg("tablespace location cannot contain single quotes")));
 
 	/*
-	 * Allowing relative paths seems risky
+	 * Allowing relative paths seems risky in general, but we'll allow it under
+	 * pg_user_files.
 	 *
 	 * this also helps us ensure that location is not empty or whitespace
 	 */
-	if (!is_absolute_path(location))
+	if (!is_absolute_path(location) &&
+		!pg_str_startswith(location, "pg_user_files/"))
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("tablespace location must be an absolute path")));
+				 errmsg("tablespace location must be an absolute path or a relative path under pg_user_files/")));
 
 	/*
 	 * Check that location isn't too long. Remember that we're going to append
@@ -587,6 +590,7 @@ DropTableSpace(DropTableSpaceStmt *stmt)
 static void
 create_tablespace_directories(const char *location, const Oid tablespaceoid)
 {
+	char		buffer[MAXPGPATH];
 	char	   *linkloc;
 	char	   *location_with_version_dir;
 	struct stat st;
@@ -651,6 +655,19 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
 	if (InRecovery)
 		remove_tablespace_symlink(linkloc);
 
+#ifndef WIN32
+	/*
+	 * If location is relative to pgdata, prefix it with ".." so that we have a
+	 * path relative to the symlink.  (Not needed for Windows, because our
+	 * symlink() wrapper will interpret it as relative to pgdata instead).
+	 */
+	if (!is_absolute_path(location))
+	{
+		snprintf(buffer, sizeof(buffer), "../%s", location);
+		location = buffer;
+	}
+#endif
+
 	/*
 	 * Create the symlink under PGDATA
 	 */
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 3c61c789e4..5ce1c28174 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -224,6 +224,7 @@ static const char *const subdirs[] = {
 	"pg_tblspc",
 	"pg_stat",
 	"pg_stat_tmp",
+	"pg_user_files",
 	"pg_xact",
 	"pg_logical",
 	"pg_logical/snapshots",
diff --git a/src/common/string.c b/src/common/string.c
index 3aa378c051..7e7e34f1f5 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -24,6 +24,15 @@
 #include "common/string.h"
 
 
+/*
+ * Returns whether the string `str' has the prefix `start'.
+ */
+bool
+pg_str_startswith(const char *str, const char *start)
+{
+	return strncmp(str, start, strlen(start)) == 0;
+}
+
 /*
  * Returns whether the string `str' has the postfix `end'.
  */
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 8eb5271ec8..ddd384f6f9 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -21,6 +21,7 @@ typedef struct PromptInterruptContext
 } PromptInterruptContext;
 
 /* functions in src/common/string.c */
+extern bool pg_str_startswith(const char *str, const char *start);
 extern bool pg_str_endswith(const char *str, const char *end);
 extern int	strtoint(const char *pg_restrict str, char **pg_restrict endptr,
 					 int base);
-- 
2.33.1

From 4b802e6760d31a477be285b3af71dbabb417469a Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 4 Aug 2021 17:20:48 +1200
Subject: [PATCH v6 2/4] CREATE TABLESPACE ... NEW LOCATION.

Allow tablespace directories to be created on demand.  This allows the
regression test suite to be run without knowing the path of, or having
access to, the server's data directory.

Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
 doc/src/sgml/ref/create_tablespace.sgml | 14 ++++++----
 src/backend/commands/tablespace.c       | 36 ++++++++++++++++++-------
 src/backend/parser/gram.y               | 13 ++++++---
 src/include/commands/tablespace.h       |  1 +
 src/include/nodes/parsenodes.h          |  1 +
 5 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/doc/src/sgml/ref/create_tablespace.sgml b/doc/src/sgml/ref/create_tablespace.sgml
index 84fa7ee5e2..497b2568fb 100644
--- a/doc/src/sgml/ref/create_tablespace.sgml
+++ b/doc/src/sgml/ref/create_tablespace.sgml
@@ -23,7 +23,7 @@ PostgreSQL documentation
 <synopsis>
 CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable>
     [ OWNER { <replaceable>new_owner</replaceable> | CURRENT_ROLE | CURRENT_USER | SESSION_USER } ]
-    LOCATION '<replaceable class="parameter">directory</replaceable>'
+    [ NEW ] LOCATION '<replaceable class="parameter">directory</replaceable>'
     [ WITH ( <replaceable class="parameter">tablespace_option</replaceable> = <replaceable class="parameter">value</replaceable> [, ... ] ) ]
 </synopsis>
  </refsynopsisdiv>
@@ -92,10 +92,14 @@ CREATE TABLESPACE <replaceable class="parameter">tablespace_name</replaceable>
       <listitem>
        <para>
         The directory that will be used for the tablespace. The directory
-        must exist (<command>CREATE TABLESPACE</command> will not create it),
-        should be empty, and must be owned by the
-        <productname>PostgreSQL</productname> system user.  The directory must be
-        specified by an absolute path name.
+        must exist, be empty, and be owned by the
+        <productname>PostgreSQL</productname> system user unless
+        <command>NEW</command> is specified.  If
+        <command>NEW</command> is specified it will be created, and the parent
+        directory must exist.
+        The directory must be specified by an absolute path name, or a path
+        name that is relative to the data directory and begins with
+        <literal>pg_user_files/</literal>.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index adfbde6b29..b2a1a12fbc 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -91,7 +91,8 @@ char	   *temp_tablespaces = NULL;
 
 
 static void create_tablespace_directories(const char *location,
-										  const Oid tablespaceoid);
+										  const Oid tablespaceoid,
+										  bool missing_ok);
 static bool destroy_tablespace_directories(Oid tablespaceoid, bool redo);
 
 
@@ -369,13 +370,14 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
 	/* Post creation hook for new tablespace */
 	InvokeObjectPostCreateHook(TableSpaceRelationId, tablespaceoid, 0);
 
-	create_tablespace_directories(location, tablespaceoid);
+	create_tablespace_directories(location, tablespaceoid, stmt->missing_ok);
 
 	/* Record the filesystem change in XLOG */
 	{
 		xl_tblspc_create_rec xlrec;
 
 		xlrec.ts_id = tablespaceoid;
+		xlrec.ts_missing_ok = stmt->missing_ok;
 
 		XLogBeginInsert();
 		XLogRegisterData((char *) &xlrec,
@@ -588,7 +590,8 @@ DropTableSpace(DropTableSpaceStmt *stmt)
  *	to the specified directory
  */
 static void
-create_tablespace_directories(const char *location, const Oid tablespaceoid)
+create_tablespace_directories(const char *location, const Oid tablespaceoid,
+							  bool missing_ok)
 {
 	char		buffer[MAXPGPATH];
 	char	   *linkloc;
@@ -601,16 +604,28 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
 
 	/*
 	 * Attempt to coerce target directory to safe permissions.  If this fails,
-	 * it doesn't exist or has the wrong owner.
+	 * it doesn't exist or has the wrong owner.  If NEW LOCATION, we'll create
+	 * the directory.
 	 */
 	if (chmod(location, pg_dir_create_mode) != 0)
 	{
 		if (errno == ENOENT)
-			ereport(ERROR,
-					(errcode(ERRCODE_UNDEFINED_FILE),
-					 errmsg("directory \"%s\" does not exist", location),
-					 InRecovery ? errhint("Create this directory for the tablespace before "
-										  "restarting the server.") : 0));
+		{
+			if (missing_ok)
+			{
+				if (mkdir(location, pg_dir_create_mode) != 0)
+					ereport(ERROR,
+							(errcode(ERRCODE_UNDEFINED_FILE),
+							 errmsg("could not create directory \"%s\"",
+									location)));
+			}
+			else
+				ereport(ERROR,
+						(errcode(ERRCODE_UNDEFINED_FILE),
+						 errmsg("directory \"%s\" does not exist", location),
+						 InRecovery ? errhint("Create this directory for the tablespace before "
+											  "restarting the server.") : 0));
+		}
 		else
 			ereport(ERROR,
 					(errcode_for_file_access(),
@@ -1537,8 +1552,9 @@ tblspc_redo(XLogReaderState *record)
 	{
 		xl_tblspc_create_rec *xlrec = (xl_tblspc_create_rec *) XLogRecGetData(record);
 		char	   *location = xlrec->ts_path;
+		bool		missing_ok = xlrec->ts_missing_ok;
 
-		create_tablespace_directories(location, xlrec->ts_id);
+		create_tablespace_directories(location, xlrec->ts_id, missing_ok);
 	}
 	else if (info == XLOG_TBLSPC_DROP)
 	{
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a6d0cefa6b..591b6f437f 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -578,6 +578,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <boolean> constraints_set_mode
 %type <str>		OptTableSpace OptConsTableSpace
 %type <rolespec> OptTableSpaceOwner
+%type <boolean> OptTableSpaceNew
 %type <ival>	opt_check_option
 
 %type <str>		opt_provider security_label
@@ -4559,13 +4560,14 @@ opt_procedural:
  *
  *****************************************************************************/
 
-CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner LOCATION Sconst opt_reloptions
+CreateTableSpaceStmt: CREATE TABLESPACE name OptTableSpaceOwner OptTableSpaceNew LOCATION Sconst opt_reloptions
 				{
 					CreateTableSpaceStmt *n = makeNode(CreateTableSpaceStmt);
 					n->tablespacename = $3;
 					n->owner = $4;
-					n->location = $6;
-					n->options = $7;
+					n->missing_ok = $5;
+					n->location = $7;
+					n->options = $8;
 					$$ = (Node *) n;
 				}
 		;
@@ -4574,6 +4576,11 @@ OptTableSpaceOwner: OWNER RoleSpec		{ $$ = $2; }
 			| /*EMPTY */				{ $$ = NULL; }
 		;
 
+OptTableSpaceNew:
+			NEW							{ $$ = true; }
+			| /*EMPTY*/					{ $$ = false; }
+		;
+
 /*****************************************************************************
  *
  *		QUERY :
diff --git a/src/include/commands/tablespace.h b/src/include/commands/tablespace.h
index 49cfc8479a..7a53c886c0 100644
--- a/src/include/commands/tablespace.h
+++ b/src/include/commands/tablespace.h
@@ -26,6 +26,7 @@
 typedef struct xl_tblspc_create_rec
 {
 	Oid			ts_id;
+	bool		ts_missing_ok;
 	char		ts_path[FLEXIBLE_ARRAY_MEMBER]; /* null-terminated string */
 } xl_tblspc_create_rec;
 
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 067138e6b5..fc3893f965 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2322,6 +2322,7 @@ typedef struct CreateTableSpaceStmt
 	RoleSpec   *owner;
 	char	   *location;
 	List	   *options;
+	bool		missing_ok;
 } CreateTableSpaceStmt;
 
 typedef struct DropTableSpaceStmt
-- 
2.33.1

From b165768ecec89d648159d71b79c0a0ccfd2c3b06 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 4 Aug 2021 22:17:54 +1200
Subject: [PATCH v6 3/4] Use relative paths for tablespace regression test.

Remove the machinery from pg_regress that manages the testtablespace
directory.  Instead, use a relative path.

Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
 src/test/regress/GNUmakefile                  |  3 +-
 .../tablespace.out}                           |  6 +--
 src/test/regress/pg_regress.c                 | 40 -------------------
 .../tablespace.source => sql/tablespace.sql}  |  6 +--
 src/tools/msvc/vcregress.pl                   |  2 -
 5 files changed, 7 insertions(+), 50 deletions(-)
 rename src/test/regress/{output/tablespace.source => expected/tablespace.out} (99%)
 rename src/test/regress/{input/tablespace.source => sql/tablespace.sql} (98%)

diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index fe6e0c98aa..d014b4a35d 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -119,7 +119,7 @@ submake-contrib-spi: | submake-libpgport submake-generated-headers
 ## Run tests
 ##
 
-REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 --make-testtablespace-dir \
+REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 \
 	$(EXTRA_REGRESS_OPTS)
 
 check: all
@@ -163,5 +163,4 @@ clean distclean maintainer-clean: clean-lib
 	rm -f pg_regress_main.o pg_regress.o pg_regress$(X)
 # things created by various check targets
 	rm -f $(output_files) $(input_files)
-	rm -rf testtablespace
 	rm -rf $(pg_regress_clean_files)
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/expected/tablespace.out
similarity index 99%
rename from src/test/regress/output/tablespace.source
rename to src/test/regress/expected/tablespace.out
index e7629d470e..c815901219 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/expected/tablespace.out
@@ -1,7 +1,7 @@
 -- create a tablespace using WITH clause
-CREATE TABLESPACE regress_tblspacewith LOCATION '@testtablespace@' WITH (some_nonexistent_parameter = true); -- fail
+CREATE TABLESPACE regress_tblspacewith LOCATION 'pg_user_files/testtablespace' WITH (some_nonexistent_parameter = true); -- fail
 ERROR:  unrecognized parameter "some_nonexistent_parameter"
-CREATE TABLESPACE regress_tblspacewith LOCATION '@testtablespace@' WITH (random_page_cost = 3.0); -- ok
+CREATE TABLESPACE regress_tblspacewith NEW LOCATION 'pg_user_files/testtablespace' WITH (random_page_cost = 3.0); -- ok
 -- check to see the parameter was used
 SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
        spcoptions       
@@ -12,7 +12,7 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
 -- drop the tablespace so we can re-use the location
 DROP TABLESPACE regress_tblspacewith;
 -- create a tablespace we can use
-CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@';
+CREATE TABLESPACE regress_tblspace LOCATION 'pg_user_files/testtablespace';
 -- try setting and resetting some properties for the new tablespace
 ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1);
 ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true);  -- fail
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 2c8a600bad..7c1404dac1 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -477,7 +477,6 @@ replace_string(StringInfo string, const char *replace, const char *replacement)
 static void
 convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const char *dest_subdir, const char *suffix)
 {
-	char		testtablespace[MAXPGPATH];
 	char		indir[MAXPGPATH];
 	char		outdir_sub[MAXPGPATH];
 	char	  **name;
@@ -506,9 +505,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 	if (!directory_exists(outdir_sub))
 		make_directory(outdir_sub);
 
-	/* We might need to replace @testtablespace@ */
-	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
@@ -554,7 +550,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 		{
 			replace_string(&line, "@abs_srcdir@", inputdir);
 			replace_string(&line, "@abs_builddir@", outputdir);
-			replace_string(&line, "@testtablespace@", testtablespace);
 			replace_string(&line, "@libdir@", dlpath);
 			replace_string(&line, "@DLSUFFIX@", DLSUFFIX);
 			fputs(line.data, outfile);
@@ -587,32 +582,6 @@ convert_sourcefiles(void)
 	convert_sourcefiles_in("output", outputdir, "expected", "out");
 }
 
-/*
- * Clean out the test tablespace dir, or create it if it doesn't exist.
- *
- * On Windows, doing this cleanup here makes it possible to run the
- * regression tests under a Windows administrative user account with the
- * restricted token obtained when starting pg_regress.
- */
-static void
-prepare_testtablespace_dir(void)
-{
-	char		testtablespace[MAXPGPATH];
-
-	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-
-	if (directory_exists(testtablespace))
-	{
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	}
-	make_directory(testtablespace);
-}
-
 /*
  * Scan resultmap file to find which platform-specific expected files to use.
  *
@@ -2152,7 +2121,6 @@ help(void)
 	printf(_("      --launcher=CMD            use CMD as launcher of psql\n"));
 	printf(_("      --load-extension=EXT      load the named extension before running the\n"));
 	printf(_("                                tests; can appear multiple times\n"));
-	printf(_("      --make-testtablespace-dir create testtablespace directory\n"));
 	printf(_("      --max-connections=N       maximum number of concurrent connections\n"));
 	printf(_("                                (default is 0, meaning unlimited)\n"));
 	printf(_("      --max-concurrent-tests=N  maximum number of concurrent tests in schedule\n"));
@@ -2211,12 +2179,10 @@ regression_main(int argc, char *argv[],
 		{"load-extension", required_argument, NULL, 22},
 		{"config-auth", required_argument, NULL, 24},
 		{"max-concurrent-tests", required_argument, NULL, 25},
-		{"make-testtablespace-dir", no_argument, NULL, 26},
 		{NULL, 0, NULL, 0}
 	};
 
 	bool		use_unix_sockets;
-	bool		make_testtablespace_dir = false;
 	_stringlist *sl;
 	int			c;
 	int			i;
@@ -2342,9 +2308,6 @@ regression_main(int argc, char *argv[],
 			case 25:
 				max_concurrent_tests = atoi(optarg);
 				break;
-			case 26:
-				make_testtablespace_dir = true;
-				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
@@ -2397,9 +2360,6 @@ regression_main(int argc, char *argv[],
 	unlimit_core_size();
 #endif
 
-	if (make_testtablespace_dir)
-		prepare_testtablespace_dir();
-
 	if (temp_instance)
 	{
 		FILE	   *pg_conf;
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/sql/tablespace.sql
similarity index 98%
rename from src/test/regress/input/tablespace.source
rename to src/test/regress/sql/tablespace.sql
index cb9774ecc8..a4a4067c42 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/sql/tablespace.sql
@@ -1,6 +1,6 @@
 -- create a tablespace using WITH clause
-CREATE TABLESPACE regress_tblspacewith LOCATION '@testtablespace@' WITH (some_nonexistent_parameter = true); -- fail
-CREATE TABLESPACE regress_tblspacewith LOCATION '@testtablespace@' WITH (random_page_cost = 3.0); -- ok
+CREATE TABLESPACE regress_tblspacewith LOCATION 'pg_user_files/testtablespace' WITH (some_nonexistent_parameter = true); -- fail
+CREATE TABLESPACE regress_tblspacewith NEW LOCATION 'pg_user_files/testtablespace' WITH (random_page_cost = 3.0); -- ok
 
 -- check to see the parameter was used
 SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
@@ -9,7 +9,7 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
 DROP TABLESPACE regress_tblspacewith;
 
 -- create a tablespace we can use
-CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@';
+CREATE TABLESPACE regress_tblspace LOCATION 'pg_user_files/testtablespace';
 
 -- try setting and resetting some properties for the new tablespace
 ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1);
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f3357b3292..92debf9b0f 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -118,7 +118,6 @@ sub installcheck_internal
 		"--bindir=../../../$Config/psql",
 		"--schedule=${schedule}_schedule",
 		"--max-concurrent-tests=20",
-		"--make-testtablespace-dir",
 		"--encoding=SQL_ASCII",
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
@@ -153,7 +152,6 @@ sub check
 		"--bindir=",
 		"--schedule=${schedule}_schedule",
 		"--max-concurrent-tests=20",
-		"--make-testtablespace-dir",
 		"--encoding=SQL_ASCII",
 		"--no-locale",
 		"--temp-instance=./tmp_check");
-- 
2.33.1

From 59c191d072ceced53daf2e1f237564bdbde24425 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Tue, 5 Oct 2021 21:01:02 +1300
Subject: [PATCH v6 4/4] Test replay of regression tests.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add a new TAP test under src/test/recovery to run the standard
regression tests while a streaming replica replays the WAL.  This
provides a basic workout for WAL decoding and redo code, and compares
the replicated result.

Optionally, enable (expensive) wal_consistency_checking if listed in
the env variable PG_TEST_EXTRA.

Reviewed-by: 綱川 貴之 (Takayuki Tsunakawa) <tsunakawa.ta...@fujitsu.com>
Reviewed-by: Andres Freund <and...@anarazel.de>
Reviewed-by: Andrew Dunstan <and...@dunslane.net>
Reviewed-by: Tom Lane <t...@sss.pgh.pa.us>
Reviewed-by: Anastasia Lubennikova <lubennikov...@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
 doc/src/sgml/regress.sgml                 | 11 ++++
 src/test/perl/PostgreSQL/Test/Cluster.pm  |  2 +-
 src/test/recovery/t/027_stream_regress.pl | 68 +++++++++++++++++++++++
 3 files changed, 80 insertions(+), 1 deletion(-)
 create mode 100644 src/test/recovery/t/027_stream_regress.pl

diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 724ef566e7..64fc81776d 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -289,6 +289,17 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl'
       </para>
      </listitem>
     </varlistentry>
+
+    <varlistentry>
+     <term><literal>wal_consistency_checking</literal></term>
+     <listitem>
+      <para>
+       Uses <literal>wal_consistency_checking=all</literal> while running
+       some of the tests under <filename>src/test/recovery</filename>.  Not
+       enabled by default because it is resource intensive.
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
 
    Tests for features that are not supported by the current build
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 9467a199c8..5cfa137cde 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -460,7 +460,7 @@ sub init
 		print $conf "hot_standby = on\n";
 		# conservative settings to ensure we can run multiple postmasters:
 		print $conf "shared_buffers = 1MB\n";
-		print $conf "max_connections = 10\n";
+		print $conf "max_connections = 25\n";
 		# limit disk space consumption, too:
 		print $conf "max_wal_size = 128MB\n";
 	}
diff --git a/src/test/recovery/t/027_stream_regress.pl b/src/test/recovery/t/027_stream_regress.pl
new file mode 100644
index 0000000000..fb37a21bd1
--- /dev/null
+++ b/src/test/recovery/t/027_stream_regress.pl
@@ -0,0 +1,68 @@
+# Run the standard regression tests with streaming replication
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 4;
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+
+# WAL consistency checking is resource intensive so require opt-in with the
+# PG_TEST_EXTRA environment variable.
+if ($ENV{PG_TEST_EXTRA} &&
+	$ENV{PG_TEST_EXTRA} =~ m/\bwal_consistency_checking\b/) {
+	$node_primary->append_conf('postgresql.conf',
+		'wal_consistency_checking = all');
+}
+
+$node_primary->start;
+is( $node_primary->psql(
+        'postgres',
+        qq[SELECT pg_create_physical_replication_slot('standby_1');]),
+    0,
+    'physical slot created on primary');
+my $backup_name = 'my_backup';
+
+# Take backup
+$node_primary->backup($backup_name);
+
+# Create streaming standby linking to primary
+my $node_standby_1 = PostgreSQL::Test::Cluster->new('standby_1');
+$node_standby_1->init_from_backup($node_primary, $backup_name,
+	has_streaming => 1);
+$node_standby_1->append_conf('postgresql.conf',
+    "primary_slot_name = standby_1");
+$node_standby_1->start;
+
+# Run the regression tests against the primary.
+system_or_bail("../regress/pg_regress",
+			   "--bindir=../../bin/psql",
+			   "--port=" . $node_primary->port,
+			   "--schedule=../regress/parallel_schedule",
+			   "--dlpath=../regress",
+			   "--inputdir=../regress");
+
+# Clobber all sequences with their next value, so that we don't have
+# differences between nodes due to caching.
+$node_primary->psql('regression',
+	"select setval(seqrelid, nextval(seqrelid)) from pg_sequence");
+
+# Wait for standby to catch up
+$node_primary->wait_for_catchup($node_standby_1, 'replay',
+	$node_primary->lsn('insert'));
+
+# Perform a logical dump of primary and standby, and check that they match
+command_ok(
+	[ "pg_dump", '-f', 'primary.dump', '--no-sync', '-p', $node_primary->port, 'regression' ],
+	"dump primary server");
+command_ok(
+	[ "pg_dump", '-f', 'standby.dump', '--no-sync', '-p', $node_standby_1->port, 'regression' ],
+	"dump standby server");
+command_ok(
+	[ "diff", 'primary.dump', 'standby.dump' ],
+	"compare primary and standby dumps");
+
+$node_standby_1->stop;
+$node_primary->stop;
-- 
2.33.1

Reply via email to