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

1 files changed, 63 insertions, 47 deletions

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b540ee293b6..b393f1b6c1f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8454,9 +8454,8 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			char		fullpath[MAXPGPATH + 10];
 			char		linkpath[MAXPGPATH];
 			char	   *relpath = NULL;
-			int			rllen;
-			StringInfoData escapedpath;
 			char	   *s;
+			PGFileType	de_type;
 
 			/* Skip anything that doesn't look like a tablespace */
 			if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
@@ -8464,66 +8463,83 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 
 			snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name);
 
-			/*
-			 * Skip anything that isn't a symlink/junction.  For testing only,
-			 * we sometimes use allow_in_place_tablespaces to create
-			 * directories directly under pg_tblspc, which would fail below.
-			 */
-			if (get_dirent_type(fullpath, de, false, ERROR) != PGFILETYPE_LNK)
-				continue;
+			de_type = get_dirent_type(fullpath, de, false, ERROR);
 
-			rllen = readlink(fullpath, linkpath, sizeof(linkpath));
-			if (rllen < 0)
+			if (de_type == PGFILETYPE_LNK)
 			{
-				ereport(WARNING,
-						(errmsg("could not read symbolic link \"%s\": %m",
-								fullpath)));
-				continue;
+				StringInfoData escapedpath;
+				int			rllen;
+
+				rllen = readlink(fullpath, linkpath, sizeof(linkpath));
+				if (rllen < 0)
+				{
+					ereport(WARNING,
+							(errmsg("could not read symbolic link \"%s\": %m",
+									fullpath)));
+					continue;
+				}
+				else if (rllen >= sizeof(linkpath))
+				{
+					ereport(WARNING,
+							(errmsg("symbolic link \"%s\" target is too long",
+									fullpath)));
+					continue;
+				}
+				linkpath[rllen] = '\0';
+
+				/*
+				 * Relpath holds the relative path of the tablespace directory
+				 * when it's located within PGDATA, or NULL if it's located
+				 * elsewhere.
+				 */
+				if (rllen > datadirpathlen &&
+					strncmp(linkpath, DataDir, datadirpathlen) == 0 &&
+						IS_DIR_SEP(linkpath[datadirpathlen]))
+					relpath = pstrdup(linkpath + datadirpathlen + 1);
+
+				/*
+				 * Add a backslash-escaped version of the link path to the
+				 * tablespace map file.
+				 */
+				initStringInfo(&escapedpath);
+				for (s = linkpath; *s; s++)
+				{
+					if (*s == '\n' || *s == '\r' || *s == '\\')
+						appendStringInfoChar(&escapedpath, '\\');
+					appendStringInfoChar(&escapedpath, *s);
+				}
+				appendStringInfo(tblspcmapfile, "%s %s\n",
+								 de->d_name, escapedpath.data);
+				pfree(escapedpath.data);
 			}
-			else if (rllen >= sizeof(linkpath))
+			else if (de_type == PGFILETYPE_DIR)
 			{
-				ereport(WARNING,
-						(errmsg("symbolic link \"%s\" target is too long",
-								fullpath)));
-				continue;
+				/*
+				 * It's possible to use allow_in_place_tablespaces to create
+				 * directories directly under pg_tblspc, for testing purposes
+				 * only.
+				 *
+				 * In this case, we store a relative path rather than an
+				 * absolute path into the tablespaceinfo.
+				 */
+				snprintf(linkpath, sizeof(linkpath), "pg_tblspc/%s",
+						 de->d_name);
+				relpath = pstrdup(linkpath);
 			}
-			linkpath[rllen] = '\0';
-
-			/*
-			 * Build a backslash-escaped version of the link path to include
-			 * in the tablespace map file.
-			 */
-			initStringInfo(&escapedpath);
-			for (s = linkpath; *s; s++)
+			else
 			{
-				if (*s == '\n' || *s == '\r' || *s == '\\')
-					appendStringInfoChar(&escapedpath, '\\');
-				appendStringInfoChar(&escapedpath, *s);
+				/* Skip any other file type that appears here. */
+				continue;
 			}
 
-			/*
-			 * Relpath holds the relative path of the tablespace directory
-			 * when it's located within PGDATA, or NULL if it's located
-			 * elsewhere.
-			 */
-			if (rllen > datadirpathlen &&
-				strncmp(linkpath, DataDir, datadirpathlen) == 0 &&
-				IS_DIR_SEP(linkpath[datadirpathlen]))
-				relpath = linkpath + datadirpathlen + 1;
-
 			ti = palloc(sizeof(tablespaceinfo));
 			ti->oid = pstrdup(de->d_name);
 			ti->path = pstrdup(linkpath);
-			ti->rpath = relpath ? pstrdup(relpath) : NULL;
+			ti->rpath = relpath;
 			ti->size = -1;
 
 			if (tablespaces)
 				*tablespaces = lappend(*tablespaces, ti);
-
-			appendStringInfo(tblspcmapfile, "%s %s\n",
-							 ti->oid, escapedpath.data);
-
-			pfree(escapedpath.data);
 		}
 		FreeDir(tblspcdir);