Fix pg_basebackup with in-place tablespaces some more.

Commit c6f2f01611d4f2c412e92eb7893f76fa590818e8 purported to make
this work, but problems remained. In a plain-format backup, the
files from an in-place tablespace got included in the tar file for
the main tablespace, which is wrong but it's not clear that it
has any user-visible consequences. In a tar-format backup, the
TABLESPACE_MAP option is used, and so we never iterated over
pg_tblspc and thus never backed up the in-place tablespaces
anywhere at all.

To fix this, reverse the changes in that commit, so that when we scan
pg_tblspc during a backup, we create tablespaceinfo objects even for
in-place tablespaces. We set the field that would normally contain the
absolute pathname to the relative path pg_tblspc/${TSOID}, and that's
good enough to make basebackup.c happy without any further changes.

However, pg_basebackup needs a couple of adjustments to make it work.
First, it needs to understand that a relative path for a tablespace
means it's an in-place tablespace.  Second, it needs to tolerate the
situation where restoring the main tablespace tries to create
pg_tblspc or a subdirectory and finds that it already exists, because
we restore user-defined tablespaces before the main tablespace.

Since in-place tablespaces are only intended for use in development
and testing, no back-patch.

Patch by me, reviewed by Thomas Munro and Michael Paquier.

Discussion: http://postgr.es/m/CA+TgmobwvbEp+fLq2PykMYzizcvuNv0a7gPMJtxOTMOuuRLMHg@mail.gmail.com

3 files changed, 96 insertions, 19 deletions

diff --git a/src/bin/pg_basebackup/bbstreamer_file.c b/src/bin/pg_basebackup/bbstreamer_file.c
index df4fb864fe..45f32974ff 100644
--- a/src/bin/pg_basebackup/bbstreamer_file.c
+++ b/src/bin/pg_basebackup/bbstreamer_file.c
@@ -277,27 +277,48 @@ bbstreamer_extractor_content(bbstreamer *streamer, bbstreamer_member *member,
 }
 
 /*
+ * Should we tolerate an already-existing directory?
+ *
+ * When streaming WAL, pg_wal (or pg_xlog for pre-9.6 clusters) will have been
+ * created by the wal receiver process. Also, when the WAL directory location
+ * was specified, pg_wal (or pg_xlog) has already been created as a symbolic
+ * link before starting the actual backup.  So just ignore creation failures
+ * on related directories.
+ *
+ * If in-place tablespaces are used, pg_tblspc and subdirectories may already
+ * exist when we get here. So tolerate that case, too.
+ */
+static bool
+should_allow_existing_directory(const char *pathname)
+{
+	const char *filename = last_dir_separator(pathname) + 1;
+
+	if (strcmp(filename, "pg_wal") == 0 ||
+		strcmp(filename, "pg_xlog") == 0 ||
+		strcmp(filename, "archive_status") == 0 ||
+		strcmp(filename, "pg_tblspc") == 0)
+		return true;
+
+	if (strspn(filename, "0123456789") == strlen(filename))
+	{
+		const char *pg_tblspc = strstr(pathname, "/pg_tblspc/");
+
+		return pg_tblspc != NULL && pg_tblspc + 11 == filename;
+	}
+
+	return false;
+}
+
+/*
  * Create a directory.
  */
 static void
 extract_directory(const char *filename, mode_t mode)
 {
-	if (mkdir(filename, pg_dir_create_mode) != 0)
-	{
-		/*
-		 * When streaming WAL, pg_wal (or pg_xlog for pre-9.6 clusters) will
-		 * have been created by the wal receiver process. Also, when the WAL
-		 * directory location was specified, pg_wal (or pg_xlog) has already
-		 * been created as a symbolic link before starting the actual backup.
-		 * So just ignore creation failures on related directories.
-		 */
-		if (!((pg_str_endswith(filename, "/pg_wal") ||
-			   pg_str_endswith(filename, "/pg_xlog") ||
-			   pg_str_endswith(filename, "/archive_status")) &&
-			  errno == EEXIST))
-			pg_fatal("could not create directory \"%s\": %m",
-					 filename);
-	}
+	if (mkdir(filename, pg_dir_create_mode) != 0 &&
+		(errno != EEXIST || !should_allow_existing_directory(filename)))
+		pg_fatal("could not create directory \"%s\": %m",
+				 filename);
 
 #ifndef WIN32
 	if (chmod(filename, mode))
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 948b859b56..ba471f898c 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1122,9 +1122,17 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 		 * other tablespaces will be written to the directory where they're
 		 * located on the server, after applying any user-specified tablespace
 		 * mappings.
+		 *
+		 * In the case of an in-place tablespace, spclocation will be a
+		 * relative path. We just convert it to an absolute path by prepending
+		 * basedir.
 		 */
-		directory = spclocation == NULL ? basedir
-			: get_tablespace_mapping(spclocation);
+		if (spclocation == NULL)
+			directory = basedir;
+		else if (!is_absolute_path(spclocation))
+			directory = psprintf("%s/%s", basedir, spclocation);
+		else
+			directory = get_tablespace_mapping(spclocation);
 		streamer = bbstreamer_extractor_new(directory,
 											get_tablespace_mapping,
 											progress_update_filename);
@@ -1955,7 +1963,15 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 		 */
 		if (backup_target == NULL && format == 'p' && !PQgetisnull(res, i, 1))
 		{
-			char	   *path = unconstify(char *, get_tablespace_mapping(PQgetvalue(res, i, 1)));
+			char	   *path = PQgetvalue(res, i, 1);
+
+			if (is_absolute_path(path))
+				path = unconstify(char *, get_tablespace_mapping(path));
+			else
+			{
+				/* This is an in-place tablespace, so prepend basedir. */
+				path = psprintf("%s/%s", basedir, path);
+			}
 
 			verify_dir_is_empty_or_create(path, &made_tablespace_dirs, &found_tablespace_dirs);
 		}
diff --git a/src/bin/pg_basebackup/t/011_in_place_tablespace.pl b/src/bin/pg_basebackup/t/011_in_place_tablespace.pl
new file mode 100644
index 0000000000..d58696e8f9
--- /dev/null
+++ b/src/bin/pg_basebackup/t/011_in_place_tablespace.pl
@@ -0,0 +1,40 @@
+# Copyright (c) 2021-2023, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+
+# For nearly all pg_basebackup invocations some options should be specified,
+# to keep test times reasonable. Using @pg_basebackup_defs as the first
+# element of the array passed to IPC::Run interpolate the array (as it is
+# not a reference to an array)...
+my @pg_basebackup_defs = ('pg_basebackup', '--no-sync', '-cfast');
+
+# Set up an instance.
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init('allows_streaming' => 1);
+$node->start();
+
+# Create an in-place tablespace.
+$node->safe_psql('postgres', <<EOM);
+SET allow_in_place_tablespaces = on;
+CREATE TABLESPACE inplace LOCATION '';
+EOM
+
+# Back it up.
+my $backupdir = $tempdir . '/backup';
+$node->command_ok(
+	[ @pg_basebackup_defs, '-D', $backupdir, '-Ft', '-X', 'none' ],
+	'pg_basebackup runs');
+
+# Make sure we got base.tar and one tablespace.
+ok(-f "$backupdir/base.tar", 'backup tar was created');
+my @tblspc_tars = glob "$backupdir/[0-9]*.tar";
+is(scalar(@tblspc_tars), 1, 'one tablespace tar was created');
+
+# All good.
+done_testing();