define PG_REPLSLOT_DIR

ashutosh.bapat.oss@gmail.com

over 1 year ago

In reply to: Bertrand Drouvot (#1)

Re: define PG_REPLSLOT_DIR

On Wed, Aug 14, 2024 at 5:02 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi hackers,

while working on a replication slot tool (idea is to put it in contrib, not
shared yet), I realized that "pg_replslot" is being used > 25 times in
.c files.

I think it would make sense to replace those occurrences with a $SUBJECT, attached
a patch doing so.

Many of these places are slot specific directory/file names within
pg_replslot. I think we can further improve the code by creating macro
on the lines of LSN_FORMAT_ARGS
#define SLOT_DIRNAME_ARGS(slotname) (PG_REPLSLOT_DIR, slotname)
this way we "codify" method to construct the slot directory name
everywhere. We may add another macro
#define SLOT_DIRNAME_FORMAT "%s/%s" to further enforce the same. But
I didn't find similar LSN_FORMAT macro defined as "%X/%X". I don't
remember exactly why we didn't add it. May be because of trouble with
translations.

--
Best Wishes,
Ashutosh Bapat

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Alvaro Herrera (#2)

5 attachment(s)

Re: define PG_REPLSLOT_DIR

Hi,

On Thu, Aug 15, 2024 at 08:40:42PM -0400, Alvaro Herrera wrote:

On 2024-Aug-14, Bertrand Drouvot wrote:

Out of curiosity, checking the sizes of affected files (O2, no debug):

with patch:

text data bss dec hex filename
30304 0 8 30312 7668 src/backend/replication/logical/reorderbuffer.o

without patch:
30286 0 8 30294 7656 src/backend/replication/logical/reorderbuffer.o

Hmm, I suppose this delta can be explained because because the literal
string is used inside larger snprintf() format strings or similar, so
things like
snprintf(path, sizeof(path),
-                    "pg_replslot/%s/%s", slotname,
+                    "%s/%s/%s", PG_REPLSLOT_DIR, slotname,
spill_de->d_name);
and
ereport(ERROR,
(errcode_for_file_access(),
-                        errmsg("could not remove file \"%s\" during removal of pg_replslot/%s/xid*: %m",
-                               path, slotname)));
+                        errmsg("could not remove file \"%s\" during removal of %s/%s/xid*: %m",
+                               PG_REPLSLOT_DIR, path, slotname)));

I did not look in details, but yeah that could probably be explained that way.

I don't disagree with making this change, but changing some of the other
directory names you suggest, such as

pg_notify
pg_serial
pg_subtrans
pg_multixact
pg_wal

would probably make no difference, since there are no literal strings
that contain that as a substring(*). It might some sense to handle
pg_tblspc similarly. As for pg_logical, I think you'll want separate
defines for pg_logical/mappings, pg_logical/snapshots and so on.

(*) I hope you're not going to suggest this kind of change (word-diff):

if (GetOldestSnapshot())
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("[-pg_wal-]{+%s+}_replay_wait() must be only called without an active or registered snapshot"{+, PG_WAL_DIR+}),
errdetail("Make sure [-pg_wal-]{+%s+}_replay_wait() isn't called within a transaction with an isolation level higher than READ COMMITTED, another procedure, or a function."{+, PG_WAL_DIR+})));

Yeah, would not cross my mind ;-). I might have been tempted to do the change
in pg_combinebackup.c though.

Having said that, I agree to focus only on:

pg_replslot
pg_tblspc
pg_logical/*

I made the changes for pg_tblspc in pg_combinebackup.c as the number of occurences
are greater that the "pg_wal" ones and we were to define PG_TBLSPC_DIR in any
case.

Please find attached the related patches.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v2-0003-Define-PG_LOGICAL_SNAPSHOTS_DIR.patchtext/x-diff; charset=us-asciiDownload

From a76100f2fbb1906195a18ffbffbf897e83627916 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 09:42:40 +0000
Subject: [PATCH v2 3/5] Define PG_LOGICAL_SNAPSHOTS_DIR

Replace most of the places where "pg_logical/snapshots" is used in .c files with
a new PG_LOGICAL_SNAPSHOTS_DIR define. The places where it is not done is for
consistency with the existing PG_STAT_TMP_DIR define.
---
 src/backend/replication/logical/snapbuild.c | 28 ++++++++++++---------
 src/backend/utils/adt/genfile.c             |  4 +--
 src/include/replication/reorderbuffer.h     |  1 +
 3 files changed, 19 insertions(+), 14 deletions(-)
  79.1% src/backend/replication/logical/
  17.5% src/backend/utils/adt/
   3.3% src/include/replication/

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index ae676145e6..0450f94ba8 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1654,7 +1654,8 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 	 * unless the user used pg_resetwal or similar. If a user did so, there's
 	 * no hope continuing to decode anyway.
 	 */
-	sprintf(path, "pg_logical/snapshots/%X-%X.snap",
+	sprintf(path, "%s/%X-%X.snap",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn));
 
 	/*
@@ -1681,7 +1682,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 		 * be safely on disk.
 		 */
 		fsync_fname(path, false);
-		fsync_fname("pg_logical/snapshots", true);
+		fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 		builder->last_serialized_snapshot = lsn;
 		goto out;
@@ -1697,7 +1698,8 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 	elog(DEBUG1, "serializing snapshot to %s", path);
 
 	/* to make sure only we will write to this tempfile, include pid */
-	sprintf(tmppath, "pg_logical/snapshots/%X-%X.snap.%d.tmp",
+	sprintf(tmppath, "%s/%X-%X.snap.%d.tmp",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn), MyProcPid);
 
 	/*
@@ -1818,7 +1820,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 				(errcode_for_file_access(),
 				 errmsg("could not close file \"%s\": %m", tmppath)));
 
-	fsync_fname("pg_logical/snapshots", true);
+	fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 	/*
 	 * We may overwrite the work from some other backend, but that's ok, our
@@ -1834,7 +1836,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 
 	/* make sure we persist */
 	fsync_fname(path, false);
-	fsync_fname("pg_logical/snapshots", true);
+	fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 	/*
 	 * Now there's no way we can lose the dumped state anymore, remember this
@@ -1871,7 +1873,8 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
 	if (builder->state == SNAPBUILD_CONSISTENT)
 		return false;
 
-	sprintf(path, "pg_logical/snapshots/%X-%X.snap",
+	sprintf(path, "%s/%X-%X.snap",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn));
 
 	fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
@@ -1892,7 +1895,7 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
 	 * ----
 	 */
 	fsync_fname(path, false);
-	fsync_fname("pg_logical/snapshots", true);
+	fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 
 	/* read statically sized portion of snapshot */
@@ -2074,7 +2077,7 @@ CheckPointSnapBuild(void)
 	XLogRecPtr	redo;
 	DIR		   *snap_dir;
 	struct dirent *snap_de;
-	char		path[MAXPGPATH + 21];
+	char		path[MAXPGPATH + sizeof(PG_LOGICAL_SNAPSHOTS_DIR)];
 
 	/*
 	 * We start off with a minimum of the last redo pointer. No new
@@ -2090,8 +2093,8 @@ CheckPointSnapBuild(void)
 	if (redo < cutoff)
 		cutoff = redo;
 
-	snap_dir = AllocateDir("pg_logical/snapshots");
-	while ((snap_de = ReadDir(snap_dir, "pg_logical/snapshots")) != NULL)
+	snap_dir = AllocateDir(PG_LOGICAL_SNAPSHOTS_DIR);
+	while ((snap_de = ReadDir(snap_dir, PG_LOGICAL_SNAPSHOTS_DIR)) != NULL)
 	{
 		uint32		hi;
 		uint32		lo;
@@ -2102,7 +2105,7 @@ CheckPointSnapBuild(void)
 			strcmp(snap_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(path, sizeof(path), "pg_logical/snapshots/%s", snap_de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_LOGICAL_SNAPSHOTS_DIR, snap_de->d_name);
 		de_type = get_dirent_type(path, snap_de, false, DEBUG1);
 
 		if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG)
@@ -2162,7 +2165,8 @@ SnapBuildSnapshotExists(XLogRecPtr lsn)
 	int			ret;
 	struct stat stat_buf;
 
-	sprintf(path, "pg_logical/snapshots/%X-%X.snap",
+	sprintf(path, "%s/%X-%X.snap",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn));
 
 	ret = stat(path, &stat_buf);
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index b57254286b..9f8c044eeb 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -690,12 +690,12 @@ pg_ls_archive_statusdir(PG_FUNCTION_ARGS)
 }
 
 /*
- * Function to return the list of files in the pg_logical/snapshots directory.
+ * Function to return the list of files in the PG_LOGICAL_SNAPSHOTS_DIR directory.
  */
 Datum
 pg_ls_logicalsnapdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, "pg_logical/snapshots", false);
+	return pg_ls_dir_files(fcinfo, PG_LOGICAL_SNAPSHOTS_DIR, false);
 }
 
 /*
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index bf3b8010f6..60a30763c9 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -19,6 +19,7 @@
 #include "utils/timestamp.h"
 
 #define PG_LOGICAL_MAPPINGS_DIR "pg_logical/mappings"
+#define PG_LOGICAL_SNAPSHOTS_DIR "pg_logical/snapshots"
 
 /* GUC variables */
 extern PGDLLIMPORT int logical_decoding_work_mem;
-- 
2.34.1

v2-0004-Define-PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR.patchtext/x-diff; charset=us-asciiDownload

From 214a015557573f25b5db7ae1bcec90ea92366fb0 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 09:53:03 +0000
Subject: [PATCH v2 4/5] Define PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR

Replace the places where "pg_logical/replorigin_checkpoint" is used in .c files
with a new PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR define.
---
 src/backend/replication/logical/origin.c | 6 +++---
 src/include/replication/origin.h         | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)
  67.1% src/backend/replication/logical/
  32.8% src/include/replication/

diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 419e4814f0..a4aaefa97c 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -572,8 +572,8 @@ ReplicationOriginShmemInit(void)
 void
 CheckPointReplicationOrigin(void)
 {
-	const char *tmppath = "pg_logical/replorigin_checkpoint.tmp";
-	const char *path = "pg_logical/replorigin_checkpoint";
+	const char *tmppath = PG_LOGICAL_REPLORIGIN_CHECKPOINT_TMP_DIR;
+	const char *path = PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR;
 	int			tmpfd;
 	int			i;
 	uint32		magic = REPLICATION_STATE_MAGIC;
@@ -698,7 +698,7 @@ CheckPointReplicationOrigin(void)
 void
 StartupReplicationOrigin(void)
 {
-	const char *path = "pg_logical/replorigin_checkpoint";
+	const char *path = PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR;
 	int			fd;
 	int			readBytes;
 	uint32		magic = REPLICATION_STATE_MAGIC;
diff --git a/src/include/replication/origin.h b/src/include/replication/origin.h
index 7189ba9e76..1cb44be58d 100644
--- a/src/include/replication/origin.h
+++ b/src/include/replication/origin.h
@@ -15,6 +15,9 @@
 #include "access/xlogreader.h"
 #include "catalog/pg_replication_origin.h"
 
+#define PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR "pg_logical/replorigin_checkpoint"
+#define PG_LOGICAL_REPLORIGIN_CHECKPOINT_TMP_DIR PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR ".tmp"
+
 typedef struct xl_replorigin_set
 {
 	XLogRecPtr	remote_lsn;
-- 
2.34.1

v2-0005-Define-PG_TBLSPC_DIR.patchtext/x-diff; charset=us-asciiDownload

From 46bd885f0e5ae146f7b08d08bb54747c7cee0c1c Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 07:40:10 +0000
Subject: [PATCH v2 5/5] Define PG_TBLSPC_DIR

Replace most of the places where "pg_tblspc" is used in .c files with a new
PG_TBLSPC_DIR define. The places where it is not done is for consistency with
the existing PG_STAT_TMP_DIR define.
---
 src/backend/access/transam/xlog.c           | 20 +++++------
 src/backend/access/transam/xlogrecovery.c   | 26 +++++++-------
 src/backend/backup/backup_manifest.c        |  3 +-
 src/backend/backup/basebackup.c             | 10 +++---
 src/backend/commands/dbcommands.c           |  6 ++--
 src/backend/commands/tablespace.c           | 14 ++++----
 src/backend/storage/file/fd.c               | 39 +++++++++++----------
 src/backend/storage/file/reinit.c           | 16 ++++-----
 src/backend/utils/adt/dbsize.c              |  8 ++---
 src/backend/utils/adt/misc.c                |  6 ++--
 src/backend/utils/cache/relcache.c          |  4 +--
 src/bin/pg_checksums/pg_checksums.c         |  8 ++---
 src/bin/pg_combinebackup/pg_combinebackup.c | 35 +++++++++---------
 src/bin/pg_rewind/file_ops.c                |  4 +--
 src/bin/pg_upgrade/exec.c                   |  2 +-
 src/common/file_utils.c                     |  9 ++---
 src/common/relpath.c                        | 20 +++++------
 src/fe_utils/astreamer_file.c               | 13 ++++---
 src/include/common/relpath.h                |  4 +++
 19 files changed, 129 insertions(+), 118 deletions(-)
  17.5% src/backend/access/transam/
   5.5% src/backend/backup/
   8.5% src/backend/commands/
  23.3% src/backend/storage/file/
   5.5% src/backend/utils/adt/
   3.5% src/bin/pg_checksums/
  14.8% src/bin/pg_combinebackup/
  10.6% src/common/
   5.3% src/fe_utils/
   4.9% src/

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ee0fb0e28f..48718cbdf3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5558,8 +5558,8 @@ StartupXLOG(void)
 	 * in place if the database had been cleanly shut down, but it seems
 	 * safest to just remove them always and let them be rebuilt during the
 	 * first backend startup.  These files needs to be removed from all
-	 * directories including pg_tblspc, however the symlinks are created only
-	 * after reading tablespace_map file in case of archive recovery from
+	 * directories including PG_TBLSPC_DIR, however the symlinks are created
+	 * only after reading tablespace_map file in case of archive recovery from
 	 * backup, so needs to clear old relcache files here after creating
 	 * symlinks.
 	 */
@@ -8944,10 +8944,10 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 		datadirpathlen = strlen(DataDir);
 
 		/* Collect information about all tablespaces */
-		tblspcdir = AllocateDir("pg_tblspc");
-		while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL)
+		tblspcdir = AllocateDir(PG_TBLSPC_DIR);
+		while ((de = ReadDir(tblspcdir, PG_TBLSPC_DIR)) != NULL)
 		{
-			char		fullpath[MAXPGPATH + 10];
+			char		fullpath[MAXPGPATH + sizeof(PG_TBLSPC_DIR)];
 			char		linkpath[MAXPGPATH];
 			char	   *relpath = NULL;
 			char	   *s;
@@ -8970,7 +8970,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			if (*badp != '\0' || errno == EINVAL || errno == ERANGE)
 				continue;
 
-			snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name);
+			snprintf(fullpath, sizeof(fullpath), "%s/%s", PG_TBLSPC_DIR, de->d_name);
 
 			de_type = get_dirent_type(fullpath, de, false, ERROR);
 
@@ -9025,14 +9025,14 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			{
 				/*
 				 * It's possible to use allow_in_place_tablespaces to create
-				 * directories directly under pg_tblspc, for testing purposes
-				 * only.
+				 * directories directly under PG_TBLSPC_DIR, 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);
+				snprintf(linkpath, sizeof(linkpath), "%s/%s",
+						 PG_TBLSPC_DIR, de->d_name);
 				relpath = pstrdup(linkpath);
 			}
 			else
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ad817fbca6..9ae72933d7 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -677,7 +677,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
 				tablespaceinfo *ti = lfirst(lc);
 				char	   *linkloc;
 
-				linkloc = psprintf("pg_tblspc/%u", ti->oid);
+				linkloc = psprintf("%s/%u", PG_TBLSPC_DIR, ti->oid);
 
 				/*
 				 * Remove the existing symlink if any and Create the symlink
@@ -2138,11 +2138,11 @@ xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI)
 }
 
 /*
- * Verify that, in non-test mode, ./pg_tblspc doesn't contain any real
+ * Verify that, in non-test mode, ./PG_TBLSPC_DIR doesn't contain any real
  * directories.
  *
  * Replay of database creation XLOG records for databases that were later
- * dropped can create fake directories in pg_tblspc.  By the time consistency
+ * dropped can create fake directories in PG_TBLSPC_DIR.  By the time consistency
  * is reached these directories should have been removed; here we verify
  * that this did indeed happen.  This is to be called at the point where
  * consistent state is reached.
@@ -2157,23 +2157,23 @@ CheckTablespaceDirectory(void)
 	DIR		   *dir;
 	struct dirent *de;
 
-	dir = AllocateDir("pg_tblspc");
-	while ((de = ReadDir(dir, "pg_tblspc")) != NULL)
+	dir = AllocateDir(PG_TBLSPC_DIR);
+	while ((de = ReadDir(dir, PG_TBLSPC_DIR)) != NULL)
 	{
-		char		path[MAXPGPATH + 10];
+		char		path[MAXPGPATH + sizeof(PG_TBLSPC_DIR)];
 
 		/* Skip entries of non-oid names */
 		if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
 			continue;
 
-		snprintf(path, sizeof(path), "pg_tblspc/%s", de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_TBLSPC_DIR, de->d_name);
 
 		if (get_dirent_type(path, de, false, ERROR) != PGFILETYPE_LNK)
 			ereport(allow_in_place_tablespaces ? WARNING : PANIC,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg("unexpected directory entry \"%s\" found in %s",
-							de->d_name, "pg_tblspc/"),
-					 errdetail("All directory entries in pg_tblspc/ should be symbolic links."),
+							de->d_name, PG_TBLSPC_DIR),
+					 errdetail("All directory entries in %s/ should be symbolic links.", PG_TBLSPC_DIR),
 					 errhint("Remove those directories, or set \"allow_in_place_tablespaces\" to ON transiently to let recovery complete.")));
 	}
 }
@@ -2247,10 +2247,10 @@ CheckRecoveryConsistency(void)
 		XLogCheckInvalidPages();
 
 		/*
-		 * Check that pg_tblspc doesn't contain any real directories. Replay
-		 * of Database/CREATE_* records may have created fictitious tablespace
-		 * directories that should have been removed by the time consistency
-		 * was reached.
+		 * Check that PG_TBLSPC_DIR doesn't contain any real directories.
+		 * Replay of Database/CREATE_* records may have created fictitious
+		 * tablespace directories that should have been removed by the time
+		 * consistency was reached.
 		 */
 		CheckTablespaceDirectory();
 
diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c
index 4357cfa31d..a2e2f86332 100644
--- a/src/backend/backup/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -16,6 +16,7 @@
 #include "access/xlog.h"
 #include "backup/backup_manifest.h"
 #include "backup/basebackup_sink.h"
+#include "common/relpath.h"
 #include "mb/pg_wchar.h"
 #include "utils/builtins.h"
 #include "utils/json.h"
@@ -117,7 +118,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, Oid spcoid,
 	 */
 	if (OidIsValid(spcoid))
 	{
-		snprintf(pathbuf, sizeof(pathbuf), "pg_tblspc/%u/%s", spcoid,
+		snprintf(pathbuf, sizeof(pathbuf), "%s/%u/%s", PG_TBLSPC_DIR, spcoid,
 				 pathname);
 		pathname = pathbuf;
 	}
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index de16afac74..9612fe33ce 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -1405,8 +1405,8 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 			continue;			/* don't recurse into pg_wal */
 		}
 
-		/* Allow symbolic links in pg_tblspc only */
-		if (strcmp(path, "./pg_tblspc") == 0 && S_ISLNK(statbuf.st_mode))
+		/* Allow symbolic links in PG_TBLSPC_DIR only */
+		if (strcmp(path, RELATIVE_PG_TBLSPC_DIR) == 0 && S_ISLNK(statbuf.st_mode))
 		{
 			char		linkpath[MAXPGPATH];
 			int			rllen;
@@ -1462,9 +1462,9 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 			}
 
 			/*
-			 * skip sending directories inside pg_tblspc, if not required.
+			 * skip sending directories inside PG_TBLSPC_DIR, if not required.
 			 */
-			if (strcmp(pathbuf, "./pg_tblspc") == 0 && !sendtblspclinks)
+			if (strcmp(pathbuf, RELATIVE_PG_TBLSPC_DIR) == 0 && !sendtblspclinks)
 				skip_this_dir = true;
 
 			if (!skip_this_dir)
@@ -1488,7 +1488,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 				if (OidIsValid(spcoid))
 				{
 					relspcoid = spcoid;
-					lookup_path = psprintf("pg_tblspc/%u/%s", spcoid,
+					lookup_path = psprintf("%s/%u/%s", PG_TBLSPC_DIR, spcoid,
 										   tarfilename);
 				}
 				else
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index d00ae40e19..01585232a3 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -3243,9 +3243,9 @@ database_is_invalid_oid(Oid dboid)
  * removed before the server stopped.  Since we expect that the directory will
  * be gone before reaching recovery consistency, and we have no knowledge about
  * the tablespace other than its OID here, we create a real directory under
- * pg_tblspc here instead of restoring the symlink.
+ * PG_TBLSPC_DIR here instead of restoring the symlink.
  *
- * If only_tblspc is true, then the requested directory must be in pg_tblspc/
+ * If only_tblspc is true, then the requested directory must be in PG_TBLSPC_DIR/
  */
 static void
 recovery_create_dbdir(char *path, bool only_tblspc)
@@ -3257,7 +3257,7 @@ recovery_create_dbdir(char *path, bool only_tblspc)
 	if (stat(path, &st) == 0)
 		return;
 
-	if (only_tblspc && strstr(path, "pg_tblspc/") == NULL)
+	if (only_tblspc && strstr(path, PG_TBLSPC_DIR_SLASH) == NULL)
 		elog(PANIC, "requested to created invalid directory: %s", path);
 
 	if (reachedConsistency && !allow_in_place_tablespaces)
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 113b480731..9b8f02a995 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -13,13 +13,13 @@
  * files within a tablespace into database-specific subdirectories.
  *
  * To support file access via the information given in RelFileLocator, we
- * maintain a symbolic-link map in $PGDATA/pg_tblspc. The symlinks are
+ * maintain a symbolic-link map in $PGDATA/PG_TBLSPC_DIR. The symlinks are
  * named by tablespace OIDs and point to the actual tablespace directories.
  * There is also a per-cluster version directory in each tablespace.
  * Thus the full path to an arbitrary file is
- *			$PGDATA/pg_tblspc/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
+ *			$PGDATA/PG_TBLSPC_DIR/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
  * e.g.
- *			$PGDATA/pg_tblspc/20981/PG_9.0_201002161/719849/83292814
+ *			$PGDATA/PG_TBLSPC_DIR/20981/PG_9.0_201002161/719849/83292814
  *
  * There are two tablespaces created at initdb time: pg_global (for shared
  * tables) and pg_default (for everything else).  For backwards compatibility
@@ -565,7 +565,7 @@ DropTableSpace(DropTableSpaceStmt *stmt)
 /*
  * create_tablespace_directories
  *
- *	Attempt to create filesystem infrastructure linking $PGDATA/pg_tblspc/
+ *	Attempt to create filesystem infrastructure linking $PGDATA/PG_TBLSPC_DIR/
  *	to the specified directory
  */
 static void
@@ -576,7 +576,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
 	struct stat st;
 	bool		in_place;
 
-	linkloc = psprintf("pg_tblspc/%u", tablespaceoid);
+	linkloc = psprintf("%s/%u", PG_TBLSPC_DIR, tablespaceoid);
 
 	/*
 	 * If we're asked to make an 'in place' tablespace, create the directory
@@ -692,7 +692,7 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo)
 	char	   *subfile;
 	struct stat st;
 
-	linkloc_with_version_dir = psprintf("pg_tblspc/%u/%s", tablespaceoid,
+	linkloc_with_version_dir = psprintf("%s/%u/%s", PG_TBLSPC_DIR, tablespaceoid,
 										TABLESPACE_VERSION_DIRECTORY);
 
 	/*
@@ -873,7 +873,7 @@ directory_is_empty(const char *path)
 /*
  *	remove_tablespace_symlink
  *
- * This function removes symlinks in pg_tblspc.  On Windows, junction points
+ * This function removes symlinks in PG_TBLSPC_DIR.  On Windows, junction points
  * act like directories so we must be able to apply rmdir.  This function
  * works like the symlink removal code in destroy_tablespace_directories,
  * except that failure to remove is always an ERROR.  But if the file doesn't
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 3944321ff3..bf309ba611 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1790,8 +1790,8 @@ TempTablespacePath(char *path, Oid tablespace)
 	else
 	{
 		/* All other tablespaces are accessed via symlinks */
-		snprintf(path, MAXPGPATH, "pg_tblspc/%u/%s/%s",
-				 tablespace, TABLESPACE_VERSION_DIRECTORY,
+		snprintf(path, MAXPGPATH, "%s/%u/%s/%s",
+				 PG_TBLSPC_DIR, tablespace, TABLESPACE_VERSION_DIRECTORY,
 				 PG_TEMP_FILES_DIR);
 	}
 }
@@ -3273,7 +3273,7 @@ CleanupTempFiles(bool isCommit, bool isProcExit)
 void
 RemovePgTempFiles(void)
 {
-	char		temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY) + sizeof(PG_TEMP_FILES_DIR)];
+	char		temp_path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY) + sizeof(PG_TEMP_FILES_DIR)];
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 
@@ -3287,20 +3287,21 @@ RemovePgTempFiles(void)
 	/*
 	 * Cycle through temp directories for all non-default tablespaces.
 	 */
-	spc_dir = AllocateDir("pg_tblspc");
+	spc_dir = AllocateDir(PG_TBLSPC_DIR);
 
-	while ((spc_de = ReadDirExtended(spc_dir, "pg_tblspc", LOG)) != NULL)
+	while ((spc_de = ReadDirExtended(spc_dir, PG_TBLSPC_DIR, LOG)) != NULL)
 	{
 		if (strcmp(spc_de->d_name, ".") == 0 ||
 			strcmp(spc_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY, PG_TEMP_FILES_DIR);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY,
+				 PG_TEMP_FILES_DIR);
 		RemovePgTempFilesInDir(temp_path, true, false);
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
 		RemovePgTempRelationFiles(temp_path);
 	}
 
@@ -3523,7 +3524,7 @@ do_syncfs(const char *path)
  * all potential filesystem, depending on recovery_init_sync_method setting.
  *
  * We fsync regular files and directories wherever they are, but we
- * follow symlinks only for pg_wal and immediately under pg_tblspc.
+ * follow symlinks only for pg_wal and immediately under PG_TBLSPC_DIR.
  * Other symlinks are presumed to point at files we're not responsible
  * for fsyncing, and might not have privileges to write at all.
  *
@@ -3587,15 +3588,15 @@ SyncDataDirectory(void)
 		/* Sync the top level pgdata directory. */
 		do_syncfs(".");
 		/* If any tablespaces are configured, sync each of those. */
-		dir = AllocateDir("pg_tblspc");
-		while ((de = ReadDirExtended(dir, "pg_tblspc", LOG)))
+		dir = AllocateDir(PG_TBLSPC_DIR);
+		while ((de = ReadDirExtended(dir, PG_TBLSPC_DIR, LOG)))
 		{
 			char		path[MAXPGPATH];
 
 			if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
 				continue;
 
-			snprintf(path, MAXPGPATH, "pg_tblspc/%s", de->d_name);
+			snprintf(path, MAXPGPATH, "%s/%s", PG_TBLSPC_DIR, de->d_name);
 			do_syncfs(path);
 		}
 		FreeDir(dir);
@@ -3618,7 +3619,7 @@ SyncDataDirectory(void)
 	walkdir(".", pre_sync_fname, false, DEBUG1);
 	if (xlog_is_symlink)
 		walkdir("pg_wal", pre_sync_fname, false, DEBUG1);
-	walkdir("pg_tblspc", pre_sync_fname, true, DEBUG1);
+	walkdir(PG_TBLSPC_DIR, pre_sync_fname, true, DEBUG1);
 #endif
 
 	/* Prepare to report progress syncing the data directory via fsync. */
@@ -3628,15 +3629,15 @@ SyncDataDirectory(void)
 	 * Now we do the fsync()s in the same order.
 	 *
 	 * The main call ignores symlinks, so in addition to specially processing
-	 * pg_wal if it's a symlink, pg_tblspc has to be visited separately with
-	 * process_symlinks = true.  Note that if there are any plain directories
-	 * in pg_tblspc, they'll get fsync'd twice.  That's not an expected case
-	 * so we don't worry about optimizing it.
+	 * pg_wal if it's a symlink, PG_TBLSPC_DIR has to be visited separately
+	 * with process_symlinks = true.  Note that if there are any plain
+	 * directories in PG_TBLSPC_DIR, they'll get fsync'd twice.  That's not an
+	 * expected case so we don't worry about optimizing it.
 	 */
 	walkdir(".", datadir_fsync_fname, false, LOG);
 	if (xlog_is_symlink)
 		walkdir("pg_wal", datadir_fsync_fname, false, LOG);
-	walkdir("pg_tblspc", datadir_fsync_fname, true, LOG);
+	walkdir(PG_TBLSPC_DIR, datadir_fsync_fname, true, LOG);
 }
 
 /*
diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c
index f1cd1a38d9..ee63c4e4e5 100644
--- a/src/backend/storage/file/reinit.c
+++ b/src/backend/storage/file/reinit.c
@@ -46,7 +46,7 @@ typedef struct
 void
 ResetUnloggedRelations(int op)
 {
-	char		temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)];
+	char		temp_path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY)];
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 	MemoryContext tmpctx,
@@ -77,16 +77,16 @@ ResetUnloggedRelations(int op)
 	/*
 	 * Cycle through directories for all non-default tablespaces.
 	 */
-	spc_dir = AllocateDir("pg_tblspc");
+	spc_dir = AllocateDir(PG_TBLSPC_DIR);
 
-	while ((spc_de = ReadDir(spc_dir, "pg_tblspc")) != NULL)
+	while ((spc_de = ReadDir(spc_dir, PG_TBLSPC_DIR)) != NULL)
 	{
 		if (strcmp(spc_de->d_name, ".") == 0 ||
 			strcmp(spc_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
 		ResetUnloggedRelationsInTablespaceDir(temp_path, op);
 	}
 
@@ -114,9 +114,9 @@ ResetUnloggedRelationsInTablespaceDir(const char *tsdirname, int op)
 	/*
 	 * If we get ENOENT on a tablespace directory, log it and return.  This
 	 * can happen if a previous DROP TABLESPACE crashed between removing the
-	 * tablespace directory and removing the symlink in pg_tblspc.  We don't
-	 * really want to prevent database startup in that scenario, so let it
-	 * pass instead.  Any other type of error will be reported by ReadDir
+	 * tablespace directory and removing the symlink in PG_TBLSPC_DIR.  We
+	 * don't really want to prevent database startup in that scenario, so let
+	 * it pass instead.  Any other type of error will be reported by ReadDir
 	 * (causing a startup failure).
 	 */
 	if (ts_dir == NULL && errno == ENOENT)
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index b2d9cc2792..e63e99c141 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -143,7 +143,7 @@ calculate_database_size(Oid dbOid)
 	totalsize = db_dir_size(pathname);
 
 	/* Scan the non-default tablespaces */
-	snprintf(dirpath, MAXPGPATH, "pg_tblspc");
+	snprintf(dirpath, MAXPGPATH, PG_TBLSPC_DIR);
 	dirdesc = AllocateDir(dirpath);
 
 	while ((direntry = ReadDir(dirdesc, dirpath)) != NULL)
@@ -154,8 +154,8 @@ calculate_database_size(Oid dbOid)
 			strcmp(direntry->d_name, "..") == 0)
 			continue;
 
-		snprintf(pathname, sizeof(pathname), "pg_tblspc/%s/%s/%u",
-				 direntry->d_name, TABLESPACE_VERSION_DIRECTORY, dbOid);
+		snprintf(pathname, sizeof(pathname), "%s/%s/%s/%u",
+				 PG_TBLSPC_DIR, direntry->d_name, TABLESPACE_VERSION_DIRECTORY, dbOid);
 		totalsize += db_dir_size(pathname);
 	}
 
@@ -227,7 +227,7 @@ calculate_tablespace_size(Oid tblspcOid)
 	else if (tblspcOid == GLOBALTABLESPACE_OID)
 		snprintf(tblspcPath, MAXPGPATH, "global");
 	else
-		snprintf(tblspcPath, MAXPGPATH, "pg_tblspc/%u/%s", tblspcOid,
+		snprintf(tblspcPath, MAXPGPATH, "%s/%u/%s", PG_TBLSPC_DIR, tblspcOid,
 				 TABLESPACE_VERSION_DIRECTORY);
 
 	dirdesc = AllocateDir(tblspcPath);
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 0e6c45807a..af6123d2f6 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -242,7 +242,7 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
 	if (tablespaceOid == DEFAULTTABLESPACE_OID)
 		location = "base";
 	else
-		location = psprintf("pg_tblspc/%u/%s", tablespaceOid,
+		location = psprintf("%s/%u/%s", PG_TBLSPC_DIR, tablespaceOid,
 							TABLESPACE_VERSION_DIRECTORY);
 
 	dirdesc = AllocateDir(location);
@@ -323,9 +323,9 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
 
 	/*
 	 * Find the location of the tablespace by reading the symbolic link that
-	 * is in pg_tblspc/<oid>.
+	 * is in PG_TBLSPC_DIR/<oid>.
 	 */
-	snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%u", tablespaceOid);
+	snprintf(sourcepath, sizeof(sourcepath), "%s/%u", PG_TBLSPC_DIR, tablespaceOid);
 
 	/*
 	 * Before reading the link, check if the source path is a link or a
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 66ed24e401..63efc55f09 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -6800,10 +6800,10 @@ RelationCacheInitFilePostInvalidate(void)
 void
 RelationCacheInitFileRemove(void)
 {
-	const char *tblspcdir = "pg_tblspc";
+	const char *tblspcdir = PG_TBLSPC_DIR;
 	DIR		   *dir;
 	struct dirent *de;
-	char		path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)];
+	char		path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY)];
 
 	snprintf(path, sizeof(path), "global/%s",
 			 RELCACHE_INIT_FILENAME);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index b5bb0e7887..ea2c2346ca 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -383,12 +383,12 @@ scan_directory(const char *basedir, const char *subdir, bool sizeonly)
 		else if (S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))
 		{
 			/*
-			 * If going through the entries of pg_tblspc, we assume to operate
+			 * If going through the entries of PG_TBLSPC_DIR, we assume to operate
 			 * on tablespace locations where only TABLESPACE_VERSION_DIRECTORY
 			 * is valid, resolving the linked locations and dive into them
 			 * directly.
 			 */
-			if (strncmp("pg_tblspc", subdir, strlen("pg_tblspc")) == 0)
+			if (strncmp(PG_TBLSPC_DIR, subdir, strlen(PG_TBLSPC_DIR)) == 0)
 			{
 				char		tblspc_path[MAXPGPATH];
 				struct stat tblspc_st;
@@ -593,12 +593,12 @@ main(int argc, char *argv[])
 		{
 			total_size = scan_directory(DataDir, "global", true);
 			total_size += scan_directory(DataDir, "base", true);
-			total_size += scan_directory(DataDir, "pg_tblspc", true);
+			total_size += scan_directory(DataDir, PG_TBLSPC_DIR, true);
 		}
 
 		(void) scan_directory(DataDir, "global", false);
 		(void) scan_directory(DataDir, "base", false);
-		(void) scan_directory(DataDir, "pg_tblspc", false);
+		(void) scan_directory(DataDir, PG_TBLSPC_DIR, false);
 
 		if (showprogress)
 			progress_report(true);
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 9ded5a2140..307a8c7fc1 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -318,7 +318,7 @@ main(int argc, char *argv[])
 	 * for those directories to be cleaned up on failure. In-place tablespaces
 	 * aren't handled at this stage because they're located beneath the main
 	 * output directory, and thus the cleanup of that directory will get rid
-	 * of them. Plus, the pg_tblspc directory that needs to contain them
+	 * of them. Plus, the PG_TBLSPC_DIR directory that needs to contain them
 	 * doesn't exist yet.
 	 */
 	atexit(cleanup_directories_atexit);
@@ -366,14 +366,14 @@ main(int argc, char *argv[])
 
 		/*
 		 * If it's a normal tablespace, we need to set up a symbolic link from
-		 * pg_tblspc/${OID} to the target directory; if it's an in-place
-		 * tablespace, we need to create a directory at pg_tblspc/${OID}.
+		 * PG_TBLSPC_DIR/${OID} to the target directory; if it's an in-place
+		 * tablespace, we need to create a directory at PG_TBLSPC_DIR/${OID}.
 		 */
 		if (!ts->in_place)
 		{
 			char		linkpath[MAXPGPATH];
 
-			snprintf(linkpath, MAXPGPATH, "%s/pg_tblspc/%u", opt.output,
+			snprintf(linkpath, MAXPGPATH, "%s/%s/%u", opt.output, PG_TBLSPC_DIR,
 					 ts->oid);
 
 			if (opt.dry_run)
@@ -844,8 +844,9 @@ process_directory_recursively(Oid tsoid,
 	/*
 	 * Classify this directory.
 	 *
-	 * We set is_pg_tblspc only for the toplevel pg_tblspc directory, because
-	 * the symlinks in that specific directory require special handling.
+	 * We set is_pg_tblspc only for the toplevel PG_TBLSPC_DIR directory,
+	 * because the symlinks in that specific directory require special
+	 * handling.
 	 *
 	 * We set is_pg_wal for the toplevel WAL directory and all of its
 	 * subdirectories, because those files are not included in the backup
@@ -860,19 +861,19 @@ process_directory_recursively(Oid tsoid,
 	 * strange name like INCREMENTAL.config and then complaining that
 	 * incremental backups don't work properly. The test here is a bit tricky:
 	 * incremental files occur in subdirectories of base, in pg_global itself,
-	 * and in subdirectories of pg_tblspc only if in-place tablespaces are
+	 * and in subdirectories of PG_TBLSPC_DIR only if in-place tablespaces are
 	 * used.
 	 */
 	if (OidIsValid(tsoid))
 		is_incremental_dir = true;
 	else if (relative_path != NULL)
 	{
-		is_pg_tblspc = strcmp(relative_path, "pg_tblspc") == 0;
+		is_pg_tblspc = strcmp(relative_path, PG_TBLSPC_DIR) == 0;
 		is_pg_wal = (strcmp(relative_path, "pg_wal") == 0 ||
 					 strncmp(relative_path, "pg_wal/", 7) == 0);
 		is_incremental_dir = strncmp(relative_path, "base/", 5) == 0 ||
 			strcmp(relative_path, "global") == 0 ||
-			strncmp(relative_path, "pg_tblspc/", 10) == 0;
+			strncmp(relative_path, PG_TBLSPC_DIR_SLASH, 10) == 0;
 	}
 
 	/*
@@ -895,7 +896,7 @@ process_directory_recursively(Oid tsoid,
 		strlcpy(ifulldir, input_directory, MAXPGPATH);
 		strlcpy(ofulldir, output_directory, MAXPGPATH);
 		if (OidIsValid(tsoid))
-			snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/", tsoid);
+			snprintf(manifest_prefix, MAXPGPATH, "%s/%u/", PG_TBLSPC_DIR, tsoid);
 		else
 			manifest_prefix[0] = '\0';
 	}
@@ -906,8 +907,8 @@ process_directory_recursively(Oid tsoid,
 		snprintf(ofulldir, MAXPGPATH, "%s/%s", output_directory,
 				 relative_path);
 		if (OidIsValid(tsoid))
-			snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/%s/",
-					 tsoid, relative_path);
+			snprintf(manifest_prefix, MAXPGPATH, "%s/%u/%s/",
+					 PG_TBLSPC_DIR, tsoid, relative_path);
 		else
 			snprintf(manifest_prefix, MAXPGPATH, "%s/", relative_path);
 	}
@@ -956,7 +957,7 @@ process_directory_recursively(Oid tsoid,
 			exit(1);
 
 		/*
-		 * If we're processing pg_tblspc, then check whether the filename
+		 * If we're processing PG_TBLSPC_DIR, then check whether the filename
 		 * looks like it could be a tablespace OID. If so, and if the
 		 * directory entry is a symbolic link or a directory, skip it.
 		 *
@@ -1235,7 +1236,7 @@ reset_directory_cleanup_list(void)
 }
 
 /*
- * Scan the pg_tblspc directory of the final input backup to get a canonical
+ * Scan the PG_TBLSPC_DIR directory of the final input backup to get a canonical
  * list of what tablespaces are part of the backup.
  *
  * 'pathname' should be the path to the toplevel backup directory for the
@@ -1249,7 +1250,7 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 	struct dirent *de;
 	cb_tablespace *tslist = NULL;
 
-	snprintf(pg_tblspc, MAXPGPATH, "%s/pg_tblspc", pathname);
+	snprintf(pg_tblspc, MAXPGPATH, "%s/%s", pathname, PG_TBLSPC_DIR);
 	pg_log_debug("scanning \"%s\"", pg_tblspc);
 
 	if ((dir = opendir(pg_tblspc)) == NULL)
@@ -1344,8 +1345,8 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 			 * we just record the paths within the data directories.
 			 */
 			snprintf(ts->old_dir, MAXPGPATH, "%s/%s", pg_tblspc, de->d_name);
-			snprintf(ts->new_dir, MAXPGPATH, "%s/pg_tblspc/%s", opt->output,
-					 de->d_name);
+			snprintf(ts->new_dir, MAXPGPATH, "%s/%s/%s", opt->output,
+					 PG_TBLSPC_DIR, de->d_name);
 			ts->in_place = true;
 		}
 
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index d93580bb41..762b60d3ab 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -448,11 +448,11 @@ recurse_dir(const char *datadir, const char *parentpath,
 			callback(path, FILE_TYPE_SYMLINK, 0, link_target);
 
 			/*
-			 * If it's a symlink within pg_tblspc, we need to recurse into it,
+			 * If it's a symlink within PG_TBLSPC_DIR, we need to recurse into it,
 			 * to process all the tablespaces.  We also follow a symlink if
 			 * it's for pg_wal.  Symlinks elsewhere are ignored.
 			 */
-			if ((parentpath && strcmp(parentpath, "pg_tblspc") == 0) ||
+			if ((parentpath && strcmp(parentpath, PG_TBLSPC_DIR) == 0) ||
 				strcmp(path, "pg_wal") == 0)
 				recurse_dir(datadir, path, callback);
 		}
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 058530ab3e..78db321ace 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -350,7 +350,7 @@ check_data_dir(ClusterInfo *cluster)
 	check_single_dir(pg_data, "global");
 	check_single_dir(pg_data, "pg_multixact");
 	check_single_dir(pg_data, "pg_subtrans");
-	check_single_dir(pg_data, "pg_tblspc");
+	check_single_dir(pg_data, PG_TBLSPC_DIR);
 	check_single_dir(pg_data, "pg_twophase");
 
 	/* pg_xlog has been renamed to pg_wal in v10 */
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 6bac537a1e..c045831bbc 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 
 #include "common/file_utils.h"
+#include "common/relpath.h"
 #ifdef FRONTEND
 #include "common/logging.h"
 #endif
@@ -87,7 +88,7 @@ do_syncfs(const char *path)
  * Synchronize PGDATA and all its contents.
  *
  * We sync regular files and directories wherever they are, but we follow
- * symlinks only for pg_wal (or pg_xlog) and immediately under pg_tblspc.
+ * symlinks only for pg_wal (or pg_xlog) and immediately under PG_TBLSPC_DIR.
  * Other symlinks are presumed to point at files we're not responsible for
  * syncing, and might not have privileges to write at all.
  *
@@ -105,7 +106,7 @@ sync_pgdata(const char *pg_data,
 	/* handle renaming of pg_xlog to pg_wal in post-10 clusters */
 	snprintf(pg_wal, MAXPGPATH, "%s/%s", pg_data,
 			 serverVersion < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal");
-	snprintf(pg_tblspc, MAXPGPATH, "%s/pg_tblspc", pg_data);
+	snprintf(pg_tblspc, MAXPGPATH, "%s/%s", pg_data, PG_TBLSPC_DIR);
 
 	/*
 	 * If pg_wal is a symlink, we'll need to recurse into it separately,
@@ -196,9 +197,9 @@ sync_pgdata(const char *pg_data,
 				 * Now we do the fsync()s in the same order.
 				 *
 				 * The main call ignores symlinks, so in addition to specially
-				 * processing pg_wal if it's a symlink, pg_tblspc has to be
+				 * processing pg_wal if it's a symlink, PG_TBLSPC_DIR has to be
 				 * visited separately with process_symlinks = true.  Note that
-				 * if there are any plain directories in pg_tblspc, they'll
+				 * if there are any plain directories in PG_TBLSPC_DIR, they'll
 				 * get fsync'd twice. That's not an expected case so we don't
 				 * worry about optimizing it.
 				 */
diff --git a/src/common/relpath.c b/src/common/relpath.c
index f54c36ef7a..426a0114f8 100644
--- a/src/common/relpath.c
+++ b/src/common/relpath.c
@@ -123,8 +123,8 @@ GetDatabasePath(Oid dbOid, Oid spcOid)
 	else
 	{
 		/* All other tablespaces are accessed via symlinks */
-		return psprintf("pg_tblspc/%u/%s/%u",
-						spcOid, TABLESPACE_VERSION_DIRECTORY, dbOid);
+		return psprintf("%s/%u/%s/%u",
+						PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY, dbOid);
 	}
 }
 
@@ -184,25 +184,25 @@ GetRelationPath(Oid dbOid, Oid spcOid, RelFileNumber relNumber,
 		if (procNumber == INVALID_PROC_NUMBER)
 		{
 			if (forkNumber != MAIN_FORKNUM)
-				path = psprintf("pg_tblspc/%u/%s/%u/%u_%s",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/%u_%s",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, relNumber,
 								forkNames[forkNumber]);
 			else
-				path = psprintf("pg_tblspc/%u/%s/%u/%u",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/%u",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, relNumber);
 		}
 		else
 		{
 			if (forkNumber != MAIN_FORKNUM)
-				path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u_%s",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/t%d_%u_%s",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, procNumber, relNumber,
 								forkNames[forkNumber]);
 			else
-				path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/t%d_%u",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, procNumber, relNumber);
 		}
 	}
diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c
index c9a030853b..9824009873 100644
--- a/src/fe_utils/astreamer_file.c
+++ b/src/fe_utils/astreamer_file.c
@@ -19,6 +19,7 @@
 
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/relpath.h"
 #include "common/string.h"
 #include "fe_utils/astreamer.h"
 
@@ -289,29 +290,31 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member,
  * 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
+ * If in-place tablespaces are used, PG_TBLSPC_DIR and subdirectories may already
  * exist when we get here. So tolerate that case, too.
  */
 static bool
 should_allow_existing_directory(const char *pathname)
 {
+#define PG_TBLSPC_DIR_CONCAT "/" PG_TBLSPC_DIR "/"
 	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, "summaries") == 0 ||
-		strcmp(filename, "pg_tblspc") == 0)
+		strcmp(filename, PG_TBLSPC_DIR) == 0)
 		return true;
 
 	if (strspn(filename, "0123456789") == strlen(filename))
 	{
-		const char *pg_tblspc = strstr(pathname, "/pg_tblspc/");
+		const char *pg_tblspc = strstr(pathname, PG_TBLSPC_DIR_CONCAT);
 
 		return pg_tblspc != NULL && pg_tblspc + 11 == filename;
 	}
 
 	return false;
+#undef PG_TBLSPC_DIR_CONCAT
 }
 
 /*
@@ -335,10 +338,10 @@ extract_directory(const char *filename, mode_t mode)
 /*
  * Create a symbolic link.
  *
- * It's most likely a link in pg_tblspc directory, to the location of a
+ * It's most likely a link in PG_TBLSPC_DIR directory, to the location of a
  * tablespace. Apply any tablespace mapping given on the command line
  * (--tablespace-mapping). (We blindly apply the mapping without checking that
- * the link really is inside pg_tblspc. We don't expect there to be other
+ * the link really is inside PG_TBLSPC_DIR. We don't expect there to be other
  * symlinks in a data directory, but if there are, you can call it an
  * undocumented feature that you can map them too.)
  */
diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h
index 6f006d5a93..a6cb091635 100644
--- a/src/include/common/relpath.h
+++ b/src/include/common/relpath.h
@@ -33,6 +33,10 @@ typedef Oid RelFileNumber;
 #define TABLESPACE_VERSION_DIRECTORY	"PG_" PG_MAJORVERSION "_" \
 									CppAsString2(CATALOG_VERSION_NO)
 
+#define PG_TBLSPC_DIR "pg_tblspc"
+#define RELATIVE_PG_TBLSPC_DIR "./" PG_TBLSPC_DIR
+#define PG_TBLSPC_DIR_SLASH PG_TBLSPC_DIR "/"
+
 /* Characters to allow for an OID in a relation path */
 #define OIDCHARS		10		/* max chars printed by %u */
 
-- 
2.34.1

v2-0001-Define-PG_REPLSLOT_DIR.patchtext/x-diff; charset=us-asciiDownload

From 5057933a8a4f49b57dfa50fe792d9f8162abf11a Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Wed, 14 Aug 2024 09:16:21 +0000
Subject: [PATCH v2 1/5] Define PG_REPLSLOT_DIR

Replace most of the places where "pg_replslot" is used in .c files with a new
PG_REPLSLOT_DIR define. The places where it is not done is for consistency with
the existing PG_STAT_TMP_DIR define.
---
 src/backend/backup/basebackup.c               |  3 +-
 .../replication/logical/reorderbuffer.c       | 15 ++++----
 src/backend/replication/slot.c                | 38 +++++++++----------
 src/backend/utils/adt/genfile.c               |  5 ++-
 src/bin/initdb/initdb.c                       |  2 +-
 src/bin/pg_rewind/filemap.c                   |  2 +-
 src/include/replication/slot.h                |  2 +
 7 files changed, 36 insertions(+), 31 deletions(-)
  24.0% src/backend/replication/logical/
  59.8% src/backend/replication/
   8.7% src/backend/utils/adt/
   4.1% src/bin/
   3.2% src/

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 01b35e26bd..de16afac74 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -36,6 +36,7 @@
 #include "port.h"
 #include "postmaster/syslogger.h"
 #include "postmaster/walsummarizer.h"
+#include "replication/slot.h"
 #include "replication/walsender.h"
 #include "replication/walsender_private.h"
 #include "storage/bufpage.h"
@@ -161,7 +162,7 @@ static const char *const excludeDirContents[] =
 	 * even if the intention is to restore to another primary. See backup.sgml
 	 * for a more detailed description.
 	 */
-	"pg_replslot",
+	PG_REPLSLOT_DIR,
 
 	/* Contents removed on startup, see dsm_cleanup_for_mmap(). */
 	PG_DYNSHMEM_DIR,
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 00a8327e77..6f98115d1e 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -4573,7 +4573,7 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname)
 	struct stat statbuf;
 	char		path[MAXPGPATH * 2 + 12];
 
-	sprintf(path, "pg_replslot/%s", slotname);
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, slotname);
 
 	/* we're only handling directories here, skip if it's not ours */
 	if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode))
@@ -4586,14 +4586,14 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname)
 		if (strncmp(spill_de->d_name, "xid", 3) == 0)
 		{
 			snprintf(path, sizeof(path),
-					 "pg_replslot/%s/%s", slotname,
+					 "%s/%s/%s", PG_REPLSLOT_DIR, slotname,
 					 spill_de->d_name);
 
 			if (unlink(path) != 0)
 				ereport(ERROR,
 						(errcode_for_file_access(),
-						 errmsg("could not remove file \"%s\" during removal of pg_replslot/%s/xid*: %m",
-								path, slotname)));
+						 errmsg("could not remove file \"%s\" during removal of %s/%s/xid*: %m",
+								PG_REPLSLOT_DIR, path, slotname)));
 		}
 	}
 	FreeDir(spill_dir);
@@ -4612,7 +4612,8 @@ ReorderBufferSerializedPath(char *path, ReplicationSlot *slot, TransactionId xid
 
 	XLogSegNoOffsetToRecPtr(segno, 0, wal_segment_size, recptr);
 
-	snprintf(path, MAXPGPATH, "pg_replslot/%s/xid-%u-lsn-%X-%X.spill",
+	snprintf(path, MAXPGPATH, "%s/%s/xid-%u-lsn-%X-%X.spill",
+			 PG_REPLSLOT_DIR,
 			 NameStr(MyReplicationSlot->data.name),
 			 xid, LSN_FORMAT_ARGS(recptr));
 }
@@ -4627,8 +4628,8 @@ StartupReorderBuffer(void)
 	DIR		   *logical_dir;
 	struct dirent *logical_de;
 
-	logical_dir = AllocateDir("pg_replslot");
-	while ((logical_de = ReadDir(logical_dir, "pg_replslot")) != NULL)
+	logical_dir = AllocateDir(PG_REPLSLOT_DIR);
+	while ((logical_de = ReadDir(logical_dir, PG_REPLSLOT_DIR)) != NULL)
 	{
 		if (strcmp(logical_de->d_name, ".") == 0 ||
 			strcmp(logical_de->d_name, "..") == 0)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index c290339af5..55e161f8c9 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -20,7 +20,7 @@
  * on standbys (to support cascading setups).  The requirement that slots be
  * usable on standbys precludes storing them in the system catalogs.
  *
- * Each replication slot gets its own directory inside the $PGDATA/pg_replslot
+ * Each replication slot gets its own directory inside the $PGDATA/PG_REPLSLOT_DIR
  * directory. Inside that directory the state file will contain the slot's
  * own data. Additional data can be stored alongside that file if required.
  * While the server is running, the state data is also cached in memory for
@@ -916,8 +916,8 @@ ReplicationSlotDropPtr(ReplicationSlot *slot)
 	LWLockAcquire(ReplicationSlotAllocationLock, LW_EXCLUSIVE);
 
 	/* Generate pathnames. */
-	sprintf(path, "pg_replslot/%s", NameStr(slot->data.name));
-	sprintf(tmppath, "pg_replslot/%s.tmp", NameStr(slot->data.name));
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(slot->data.name));
+	sprintf(tmppath, "%s/%s.tmp", PG_REPLSLOT_DIR, NameStr(slot->data.name));
 
 	/*
 	 * Rename the slot directory on disk, so that we'll no longer recognize
@@ -938,7 +938,7 @@ ReplicationSlotDropPtr(ReplicationSlot *slot)
 		 */
 		START_CRIT_SECTION();
 		fsync_fname(tmppath, true);
-		fsync_fname("pg_replslot", true);
+		fsync_fname(PG_REPLSLOT_DIR, true);
 		END_CRIT_SECTION();
 	}
 	else
@@ -1016,7 +1016,7 @@ ReplicationSlotSave(void)
 
 	Assert(MyReplicationSlot != NULL);
 
-	sprintf(path, "pg_replslot/%s", NameStr(MyReplicationSlot->data.name));
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(MyReplicationSlot->data.name));
 	SaveSlotToPath(MyReplicationSlot, path, ERROR);
 }
 
@@ -1881,7 +1881,7 @@ CheckPointReplicationSlots(bool is_shutdown)
 			continue;
 
 		/* save the slot to disk, locking is handled in SaveSlotToPath() */
-		sprintf(path, "pg_replslot/%s", NameStr(s->data.name));
+		sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(s->data.name));
 
 		/*
 		 * Slot's data is not flushed each time the confirmed_flush LSN is
@@ -1922,17 +1922,17 @@ StartupReplicationSlots(void)
 	elog(DEBUG1, "starting up replication slots");
 
 	/* restore all slots by iterating over all on-disk entries */
-	replication_dir = AllocateDir("pg_replslot");
-	while ((replication_de = ReadDir(replication_dir, "pg_replslot")) != NULL)
+	replication_dir = AllocateDir(PG_REPLSLOT_DIR);
+	while ((replication_de = ReadDir(replication_dir, PG_REPLSLOT_DIR)) != NULL)
 	{
-		char		path[MAXPGPATH + 12];
+		char		path[MAXPGPATH + sizeof(PG_REPLSLOT_DIR)];
 		PGFileType	de_type;
 
 		if (strcmp(replication_de->d_name, ".") == 0 ||
 			strcmp(replication_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(path, sizeof(path), "pg_replslot/%s", replication_de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_REPLSLOT_DIR, replication_de->d_name);
 		de_type = get_dirent_type(path, replication_de, false, DEBUG1);
 
 		/* we're only creating directories here, skip if it's not our's */
@@ -1949,7 +1949,7 @@ StartupReplicationSlots(void)
 								path)));
 				continue;
 			}
-			fsync_fname("pg_replslot", true);
+			fsync_fname(PG_REPLSLOT_DIR, true);
 			continue;
 		}
 
@@ -1987,8 +1987,8 @@ CreateSlotOnDisk(ReplicationSlot *slot)
 	 * takes out the lock, if we'd take the lock here, we'd deadlock.
 	 */
 
-	sprintf(path, "pg_replslot/%s", NameStr(slot->data.name));
-	sprintf(tmppath, "pg_replslot/%s.tmp", NameStr(slot->data.name));
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(slot->data.name));
+	sprintf(tmppath, "%s/%s.tmp", PG_REPLSLOT_DIR, NameStr(slot->data.name));
 
 	/*
 	 * It's just barely possible that some previous effort to create or drop a
@@ -2027,7 +2027,7 @@ CreateSlotOnDisk(ReplicationSlot *slot)
 	START_CRIT_SECTION();
 
 	fsync_fname(path, true);
-	fsync_fname("pg_replslot", true);
+	fsync_fname(PG_REPLSLOT_DIR, true);
 
 	END_CRIT_SECTION();
 }
@@ -2170,7 +2170,7 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
 
 	fsync_fname(path, false);
 	fsync_fname(dir, true);
-	fsync_fname("pg_replslot", true);
+	fsync_fname(PG_REPLSLOT_DIR, true);
 
 	END_CRIT_SECTION();
 
@@ -2195,8 +2195,8 @@ RestoreSlotFromDisk(const char *name)
 {
 	ReplicationSlotOnDisk cp;
 	int			i;
-	char		slotdir[MAXPGPATH + 12];
-	char		path[MAXPGPATH + 22];
+	char		slotdir[MAXPGPATH + sizeof(PG_REPLSLOT_DIR)];
+	char		path[MAXPGPATH + sizeof(PG_REPLSLOT_DIR) + 10];
 	int			fd;
 	bool		restored = false;
 	int			readBytes;
@@ -2205,7 +2205,7 @@ RestoreSlotFromDisk(const char *name)
 	/* no need to lock here, no concurrent access allowed yet */
 
 	/* delete temp file if it exists */
-	sprintf(slotdir, "pg_replslot/%s", name);
+	sprintf(slotdir, "%s/%s", PG_REPLSLOT_DIR, name);
 	sprintf(path, "%s/state.tmp", slotdir);
 	if (unlink(path) < 0 && errno != ENOENT)
 		ereport(PANIC,
@@ -2332,7 +2332,7 @@ RestoreSlotFromDisk(const char *name)
 					(errmsg("could not remove directory \"%s\"",
 							slotdir)));
 		}
-		fsync_fname("pg_replslot", true);
+		fsync_fname(PG_REPLSLOT_DIR, true);
 		return;
 	}
 
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 0d82557417..e645e6b85a 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -708,7 +708,7 @@ pg_ls_logicalmapdir(PG_FUNCTION_ARGS)
 }
 
 /*
- * Function to return the list of files in the pg_replslot/<replication_slot>
+ * Function to return the list of files in the PG_REPLSLOT_DIR/<replication_slot>
  * directory.
  */
 Datum
@@ -728,6 +728,7 @@ pg_ls_replslotdir(PG_FUNCTION_ARGS)
 				 errmsg("replication slot \"%s\" does not exist",
 						slotname)));
 
-	snprintf(path, sizeof(path), "pg_replslot/%s", slotname);
+	snprintf(path, sizeof(path), "%s/%s", PG_REPLSLOT_DIR, slotname);
+
 	return pg_ls_dir_files(fcinfo, path, false);
 }
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index f00718a015..2ee55eced0 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -242,7 +242,7 @@ static const char *const subdirs[] = {
 	"pg_multixact/offsets",
 	"base",
 	"base/1",
-	"pg_replslot",
+	"pg_replslot",				/* defined as PG_REPLSLOT_DIR */
 	"pg_tblspc",
 	"pg_stat",
 	"pg_stat_tmp",
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 4458324c9d..00e644d988 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -97,7 +97,7 @@ static const char *const excludeDirContents[] =
 	 * even if the intention is to restore to another primary. See backup.sgml
 	 * for a more detailed description.
 	 */
-	"pg_replslot",
+	"pg_replslot",				/* defined as PG_REPLSLOT_DIR */
 
 	/* Contents removed on startup, see dsm_cleanup_for_mmap(). */
 	"pg_dynshmem",				/* defined as PG_DYNSHMEM_DIR */
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index c2ee149fd6..9665faac1c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -17,6 +17,8 @@
 #include "storage/spin.h"
 #include "replication/walreceiver.h"
 
+#define PG_REPLSLOT_DIR     "pg_replslot"
+
 /*
  * Behaviour of replication slots, upon release or crash.
  *
-- 
2.34.1

v2-0002-Define-PG_LOGICAL_MAPPINGS_DIR.patchtext/x-diff; charset=us-asciiDownload

From 2cc59f7da6acb507587ca7c56732797a79e7112e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 09:30:16 +0000
Subject: [PATCH v2 2/5] Define PG_LOGICAL_MAPPINGS_DIR

Replace most of the places where "pg_logical/mappings" is used in .c files with
a new PG_LOGICAL_MAPPINGS_DIR define. The places where it is not done is for
consistency with the existing PG_STAT_TMP_DIR define.
---
 src/backend/access/heap/rewriteheap.c          | 18 +++++++++---------
 .../replication/logical/reorderbuffer.c        |  6 +++---
 src/backend/utils/adt/genfile.c                |  4 ++--
 src/include/replication/reorderbuffer.h        |  2 ++
 4 files changed, 16 insertions(+), 14 deletions(-)
  57.1% src/backend/access/heap/
  22.0% src/backend/replication/logical/
  17.4% src/backend/utils/adt/
   3.3% src/include/replication/

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 473f3aa9be..09ef220449 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -961,8 +961,8 @@ logical_rewrite_log_mapping(RewriteState state, TransactionId xid,
 			dboid = MyDatabaseId;
 
 		snprintf(path, MAXPGPATH,
-				 "pg_logical/mappings/" LOGICAL_REWRITE_FORMAT,
-				 dboid, relid,
+				 "%s/" LOGICAL_REWRITE_FORMAT,
+				 PG_LOGICAL_MAPPINGS_DIR, dboid, relid,
 				 LSN_FORMAT_ARGS(state->rs_begin_lsn),
 				 xid, GetCurrentTransactionId());
 
@@ -1081,8 +1081,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r)
 	xlrec = (xl_heap_rewrite_mapping *) XLogRecGetData(r);
 
 	snprintf(path, MAXPGPATH,
-			 "pg_logical/mappings/" LOGICAL_REWRITE_FORMAT,
-			 xlrec->mapped_db, xlrec->mapped_rel,
+			 "%s/" LOGICAL_REWRITE_FORMAT,
+			 PG_LOGICAL_MAPPINGS_DIR, xlrec->mapped_db, xlrec->mapped_rel,
 			 LSN_FORMAT_ARGS(xlrec->start_lsn),
 			 xlrec->mapped_xid, XLogRecGetXid(r));
 
@@ -1158,7 +1158,7 @@ CheckPointLogicalRewriteHeap(void)
 	XLogRecPtr	redo;
 	DIR		   *mappings_dir;
 	struct dirent *mapping_de;
-	char		path[MAXPGPATH + 20];
+	char		path[MAXPGPATH + sizeof(PG_LOGICAL_MAPPINGS_DIR)];
 
 	/*
 	 * We start of with a minimum of the last redo pointer. No new decoding
@@ -1173,8 +1173,8 @@ CheckPointLogicalRewriteHeap(void)
 	if (cutoff != InvalidXLogRecPtr && redo < cutoff)
 		cutoff = redo;
 
-	mappings_dir = AllocateDir("pg_logical/mappings");
-	while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL)
+	mappings_dir = AllocateDir(PG_LOGICAL_MAPPINGS_DIR);
+	while ((mapping_de = ReadDir(mappings_dir, PG_LOGICAL_MAPPINGS_DIR)) != NULL)
 	{
 		Oid			dboid;
 		Oid			relid;
@@ -1189,7 +1189,7 @@ CheckPointLogicalRewriteHeap(void)
 			strcmp(mapping_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_LOGICAL_MAPPINGS_DIR, mapping_de->d_name);
 		de_type = get_dirent_type(path, mapping_de, false, DEBUG1);
 
 		if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG)
@@ -1249,5 +1249,5 @@ CheckPointLogicalRewriteHeap(void)
 	FreeDir(mappings_dir);
 
 	/* persist directory entries to disk */
-	fsync_fname("pg_logical/mappings", true);
+	fsync_fname(PG_LOGICAL_MAPPINGS_DIR, true);
 }
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 6f98115d1e..8e59ea9801 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -5057,7 +5057,7 @@ ApplyLogicalMappingFile(HTAB *tuplecid_data, Oid relid, const char *fname)
 	int			readBytes;
 	LogicalRewriteMappingData map;
 
-	sprintf(path, "pg_logical/mappings/%s", fname);
+	sprintf(path, "%s/%s", PG_LOGICAL_MAPPINGS_DIR, fname);
 	fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
 	if (fd < 0)
 		ereport(ERROR,
@@ -5173,8 +5173,8 @@ UpdateLogicalMappings(HTAB *tuplecid_data, Oid relid, Snapshot snapshot)
 	ListCell   *file;
 	Oid			dboid = IsSharedRelation(relid) ? InvalidOid : MyDatabaseId;
 
-	mapping_dir = AllocateDir("pg_logical/mappings");
-	while ((mapping_de = ReadDir(mapping_dir, "pg_logical/mappings")) != NULL)
+	mapping_dir = AllocateDir(PG_LOGICAL_MAPPINGS_DIR);
+	while ((mapping_de = ReadDir(mapping_dir, PG_LOGICAL_MAPPINGS_DIR)) != NULL)
 	{
 		Oid			f_dboid;
 		Oid			f_relid;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index e645e6b85a..b57254286b 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -699,12 +699,12 @@ pg_ls_logicalsnapdir(PG_FUNCTION_ARGS)
 }
 
 /*
- * Function to return the list of files in the pg_logical/mappings directory.
+ * Function to return the list of files in the PG_LOGICAL_MAPPINGS_DIR directory.
  */
 Datum
 pg_ls_logicalmapdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, "pg_logical/mappings", false);
+	return pg_ls_dir_files(fcinfo, PG_LOGICAL_MAPPINGS_DIR, false);
 }
 
 /*
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index 851a001c8b..bf3b8010f6 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -18,6 +18,8 @@
 #include "utils/snapshot.h"
 #include "utils/timestamp.h"
 
+#define PG_LOGICAL_MAPPINGS_DIR "pg_logical/mappings"
+
 /* GUC variables */
 extern PGDLLIMPORT int logical_decoding_work_mem;
 extern PGDLLIMPORT int debug_logical_replication_streaming;
-- 
2.34.1

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Yugo Nagata (#3)

Re: define PG_REPLSLOT_DIR

Hi,

On Fri, Aug 16, 2024 at 01:31:11PM +0900, Yugo Nagata wrote:

On Wed, 14 Aug 2024 11:32:14 +0000
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

Looking forward to your feedback,
/* restore all slots by iterating over all on-disk entries */
-   replication_dir = AllocateDir("pg_replslot");
-   while ((replication_de = ReadDir(replication_dir, "pg_replslot")) != NULL)
+   replication_dir = AllocateDir(PG_REPLSLOT_DIR);
+   while ((replication_de = ReadDir(replication_dir, PG_REPLSLOT_DIR)) != NULL)
{
char        path[MAXPGPATH + 12];
PGFileType  de_type;
I think the size of path can be rewritten to "MAXPGPATH + sizeof(PG_REPLSLOT_DIR)"
and it seems easier to understand the reason why this size is used.

Thanks for the feedback!

Yeah, fully agree, it has been done that way in v2 that I just shared up-thread.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Ashutosh Bapat (#4)

Re: define PG_REPLSLOT_DIR

Hi,

On Mon, Aug 19, 2024 at 04:11:31PM +0530, Ashutosh Bapat wrote:

On Wed, Aug 14, 2024 at 5:02 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi hackers,

while working on a replication slot tool (idea is to put it in contrib, not
shared yet), I realized that "pg_replslot" is being used > 25 times in
.c files.

I think it would make sense to replace those occurrences with a $SUBJECT, attached
a patch doing so.

Many of these places are slot specific directory/file names within
pg_replslot. I think we can further improve the code by creating macro
on the lines of LSN_FORMAT_ARGS
#define SLOT_DIRNAME_ARGS(slotname) (PG_REPLSLOT_DIR, slotname)
this way we "codify" method to construct the slot directory name
everywhere.

Thanks for the feedback!

I think that could make sense. As the already proposed mechanical changes are
error prone (from my point of view), I would suggest to have a look at your
proposal once the proposed changes go in.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

ashutosh.bapat.oss@gmail.com

over 1 year ago

In reply to: Bertrand Drouvot (#7)

Re: define PG_REPLSLOT_DIR

On Mon, Aug 19, 2024 at 7:47 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Mon, Aug 19, 2024 at 04:11:31PM +0530, Ashutosh Bapat wrote:

On Wed, Aug 14, 2024 at 5:02 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi hackers,

while working on a replication slot tool (idea is to put it in contrib, not
shared yet), I realized that "pg_replslot" is being used > 25 times in
.c files.

I think it would make sense to replace those occurrences with a $SUBJECT, attached
a patch doing so.

Many of these places are slot specific directory/file names within
pg_replslot. I think we can further improve the code by creating macro
on the lines of LSN_FORMAT_ARGS
#define SLOT_DIRNAME_ARGS(slotname) (PG_REPLSLOT_DIR, slotname)
this way we "codify" method to construct the slot directory name
everywhere.

Thanks for the feedback!

I think that could make sense. As the already proposed mechanical changes are
error prone (from my point of view), I would suggest to have a look at your
proposal once the proposed changes go in.

What do you mean by error prone?

--
Best Wishes,
Ashutosh Bapat

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Ashutosh Bapat (#8)

Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Aug 20, 2024 at 09:26:59AM +0530, Ashutosh Bapat wrote:

On Mon, Aug 19, 2024 at 7:47 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Mon, Aug 19, 2024 at 04:11:31PM +0530, Ashutosh Bapat wrote:

On Wed, Aug 14, 2024 at 5:02 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi hackers,

while working on a replication slot tool (idea is to put it in contrib, not
shared yet), I realized that "pg_replslot" is being used > 25 times in
.c files.

I think it would make sense to replace those occurrences with a $SUBJECT, attached
a patch doing so.

Many of these places are slot specific directory/file names within
pg_replslot. I think we can further improve the code by creating macro
on the lines of LSN_FORMAT_ARGS
#define SLOT_DIRNAME_ARGS(slotname) (PG_REPLSLOT_DIR, slotname)
this way we "codify" method to construct the slot directory name
everywhere.

Thanks for the feedback!

I think that could make sense. As the already proposed mechanical changes are
error prone (from my point of view), I would suggest to have a look at your
proposal once the proposed changes go in.

What do you mean by error prone?

I meant to say that it is "easy" to make mistakes when doing those manual
mechanical changes. Also I think it's not that fun/easy to review, that's why
I think it's better to do one change at a time. Does that make sense to you?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#10

ashutosh.bapat.oss@gmail.com

over 1 year ago

In reply to: Bertrand Drouvot (#9)

Re: define PG_REPLSLOT_DIR

On Tue, Aug 20, 2024 at 10:49 AM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Tue, Aug 20, 2024 at 09:26:59AM +0530, Ashutosh Bapat wrote:

On Mon, Aug 19, 2024 at 7:47 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Mon, Aug 19, 2024 at 04:11:31PM +0530, Ashutosh Bapat wrote:

On Wed, Aug 14, 2024 at 5:02 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi hackers,

while working on a replication slot tool (idea is to put it in contrib, not
shared yet), I realized that "pg_replslot" is being used > 25 times in
.c files.

I think it would make sense to replace those occurrences with a $SUBJECT, attached
a patch doing so.

Many of these places are slot specific directory/file names within
pg_replslot. I think we can further improve the code by creating macro
on the lines of LSN_FORMAT_ARGS
#define SLOT_DIRNAME_ARGS(slotname) (PG_REPLSLOT_DIR, slotname)
this way we "codify" method to construct the slot directory name
everywhere.

Thanks for the feedback!

I think that could make sense. As the already proposed mechanical changes are
error prone (from my point of view), I would suggest to have a look at your
proposal once the proposed changes go in.

What do you mean by error prone?

I meant to say that it is "easy" to make mistakes when doing those manual
mechanical changes. Also I think it's not that fun/easy to review, that's why
I think it's better to do one change at a time. Does that make sense to you?

Since these are all related changes, doing them at once might make it
faster. You may use multiple commits (one for each change) to
"contain" errors.

--
Best Wishes,
Ashutosh Bapat

#11

michael@paquier.xyz

over 1 year ago

In reply to: Ashutosh Bapat (#10)

Re: define PG_REPLSLOT_DIR

On Tue, Aug 20, 2024 at 11:10:46AM +0530, Ashutosh Bapat wrote:

Since these are all related changes, doing them at once might make it
faster. You may use multiple commits (one for each change)

Doing multiple commits with individual definitions for each path would
be the way to go for me. All that is mechanical, still that feels
slightly cleaner.

to "contain" errors.

I am not sure what you mean by that.
--
Michael

#12

michael@paquier.xyz

over 1 year ago

In reply to: Bertrand Drouvot (#5)

Re: define PG_REPLSLOT_DIR

On Mon, Aug 19, 2024 at 02:11:55PM +0000, Bertrand Drouvot wrote:

I made the changes for pg_tblspc in pg_combinebackup.c as the number of occurences
are greater that the "pg_wal" ones and we were to define PG_TBLSPC_DIR in any
case.

Please find attached the related patches.

No real objection about the replslot and pg_logical bits.

- *            $PGDATA/pg_tblspc/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
+ *            $PGDATA/PG_TBLSPC_DIR/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber

For the tablespace parts, I am not sure that I would update the
comments to reflect the variables, TBH. Somebody reading the comments
would need to refer back to pg_tblspc/ in the header.
--
Michael

#13

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Michael Paquier (#11)

Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Aug 20, 2024 at 05:41:48PM +0900, Michael Paquier wrote:

On Tue, Aug 20, 2024 at 11:10:46AM +0530, Ashutosh Bapat wrote:

Since these are all related changes, doing them at once might make it
faster. You may use multiple commits (one for each change)

Doing multiple commits with individual definitions for each path would
be the way to go for me. All that is mechanical, still that feels
slightly cleaner.

Right, that's what v2 has done. If there is a need for v3 then I'll add one
dedicated patch for Ashutosh's proposal in the v3 series.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#14

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Michael Paquier (#12)

Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Aug 20, 2024 at 05:47:57PM +0900, Michael Paquier wrote:

On Mon, Aug 19, 2024 at 02:11:55PM +0000, Bertrand Drouvot wrote:

I made the changes for pg_tblspc in pg_combinebackup.c as the number of occurences
are greater that the "pg_wal" ones and we were to define PG_TBLSPC_DIR in any
case.

Please find attached the related patches.

No real objection about the replslot and pg_logical bits.

Thanks for looking at it!

- *            $PGDATA/pg_tblspc/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
+ *            $PGDATA/PG_TBLSPC_DIR/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
For the tablespace parts, I am not sure that I would update the
comments to reflect the variables, TBH. Somebody reading the comments
would need to refer back to pg_tblspc/ in the header.

I'm not sure as, for example, for PG_STAT_TMP_DIR we have those ones:

src/backend/backup/basebackup.c: * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
src/bin/pg_rewind/filemap.c: * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped

so I thought it would be better to be consistent.

That said, I don't have a strong opinion about it, but then I guess you'd want to
do the same for the ones related to replslot:

src/backend/replication/slot.c: * Each replication slot gets its own directory inside the $PGDATA/PG_REPLSLOT_DIR
src/backend/utils/adt/genfile.c: * Function to return the list of files in the PG_REPLSLOT_DIR/<replication_slot>

and pg_logical:

src/backend/utils/adt/genfile.c: * Function to return the list of files in the PG_LOGICAL_SNAPSHOTS_DIR directory.
src/backend/utils/adt/genfile.c: * Function to return the list of files in the PG_LOGICAL_MAPPINGS_DIR directory.

, right?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#15

Yugo Nagata

nagata@sraoss.co.jp

over 1 year ago

In reply to: Michael Paquier (#12)

Re: define PG_REPLSLOT_DIR

On Tue, 20 Aug 2024 17:47:57 +0900
Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Aug 19, 2024 at 02:11:55PM +0000, Bertrand Drouvot wrote:

I made the changes for pg_tblspc in pg_combinebackup.c as the number of occurences
are greater that the "pg_wal" ones and we were to define PG_TBLSPC_DIR in any
case.

Please find attached the related patches.

No real objection about the replslot and pg_logical bits.
- *            $PGDATA/pg_tblspc/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
+ *            $PGDATA/PG_TBLSPC_DIR/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
For the tablespace parts, I am not sure that I would update the
comments to reflect the variables, TBH. Somebody reading the comments
would need to refer back to pg_tblspc/ in the header.

I also think that it is not necessary to change the comments even for pg_replslot.

- * Each replication slot gets its own directory inside the $PGDATA/pg_replslot
+ * Each replication slot gets its own directory inside the $PGDATA/PG_REPLSLOT_DIR

For example, I found that comments in xlog.c use "pg_wal" even though XLOGDIR is used
in the codes as below, and I don't feel any problem for this.

static void
ValidateXLOGDirectoryStructure(void)
{
char path[MAXPGPATH];
struct stat stat_buf;

/* Check for pg_wal; if it doesn't exist, error out */
if (stat(XLOGDIR, &stat_buf) != 0 ||
!S_ISDIR(stat_buf.st_mode))

Should be the follwing also rewritten using sizeof(PG_REPLSLOT_DIR)?

struct stat statbuf;
char path[MAXPGPATH * 2 + 12];

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#16

Alvaro Herrera

alvherre@alvh.no-ip.org

over 1 year ago

In reply to: Bertrand Drouvot (#5)

Re: define PG_REPLSLOT_DIR

On 2024-Aug-19, Bertrand Drouvot wrote:

diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h
index 6f006d5a93..a6cb091635 100644
--- a/src/include/common/relpath.h
+++ b/src/include/common/relpath.h
@@ -33,6 +33,10 @@ typedef Oid RelFileNumber;
#define TABLESPACE_VERSION_DIRECTORY	"PG_" PG_MAJORVERSION "_" \
CppAsString2(CATALOG_VERSION_NO)

+#define PG_TBLSPC_DIR "pg_tblspc"

This one is missing some commentary along the lines of "This must not be
changed, unless you want to break every tool in the universe". As is,
it's quite tempting.

+#define PG_TBLSPC_DIR_SLASH PG_TBLSPC_DIR "/"

I would make this simply "pg_tblspc/", since it's not really possible to
change pg_tblspc anyway. Also, have a comment explaining why we have
it.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Los dioses no protegen a los insensatos. Éstos reciben protección de
otros insensatos mejor dotados" (Luis Wu, Mundo Anillo)

#17

[0]: /messages/by-id/ZsRYPcOtoqbWzjGG@paquier.xyz
[1]: /messages/by-id/20240820213048.207aade6a75e0dc1fe4d1067@sraoss.co.jp
[2]: /messages/by-id/CAExHW5vkjxuvyQ1fPPnuDW4nAT5jqox09ie36kciOV2+rhjbHA@mail.gmail.com

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Alvaro Herrera (#16)

6 attachment(s)

Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Aug 20, 2024 at 10:15:44AM -0400, Alvaro Herrera wrote:

On 2024-Aug-19, Bertrand Drouvot wrote:
diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h
index 6f006d5a93..a6cb091635 100644
--- a/src/include/common/relpath.h
+++ b/src/include/common/relpath.h
@@ -33,6 +33,10 @@ typedef Oid RelFileNumber;
#define TABLESPACE_VERSION_DIRECTORY	"PG_" PG_MAJORVERSION "_" \
CppAsString2(CATALOG_VERSION_NO)
+#define PG_TBLSPC_DIR "pg_tblspc"
This one is missing some commentary along the lines of "This must not be
changed, unless you want to break every tool in the universe". As is,
it's quite tempting.

Yeah, makes sense, thanks.

+#define PG_TBLSPC_DIR_SLASH PG_TBLSPC_DIR "/"

I would make this simply "pg_tblspc/", since it's not really possible to
change pg_tblspc anyway. Also, have a comment explaining why we have
it.

Please find attached v3 that:

- takes care of your comments (and also removed the use of PG_TBLSPC_DIR in
RELATIVE_PG_TBLSPC_DIR).
- removes the new macros from the comments (see Michael's and Yugo-San's
comments in [0]/messages/by-id/ZsRYPcOtoqbWzjGG@paquier.xyz resp. [1]/messages/by-id/20240820213048.207aade6a75e0dc1fe4d1067@sraoss.co.jp).
- adds a missing sizeof() (see [1]/messages/by-id/20240820213048.207aade6a75e0dc1fe4d1067@sraoss.co.jp).
- implements Ashutosh's idea of adding a new SLOT_DIRNAME_ARGS (see [2]/messages/by-id/CAExHW5vkjxuvyQ1fPPnuDW4nAT5jqox09ie36kciOV2+rhjbHA@mail.gmail.com). It's
done in 0002 (I used REPLSLOT_DIR_ARGS though).
- fixed a macro usage in ReorderBufferCleanupSerializedTXNs() (was not at the
right location, discovered while implementing 0002).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v3-0001-Define-PG_REPLSLOT_DIR.patchtext/x-diff; charset=us-asciiDownload

From 12398b01fccb7437c4fb35282c576c317b9a7578 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Wed, 14 Aug 2024 09:16:21 +0000
Subject: [PATCH v3 1/6] Define PG_REPLSLOT_DIR

Replace most of the places where "pg_replslot" is used in .c files with a new
PG_REPLSLOT_DIR define. The places where it is not done is for consistency with
the existing PG_STAT_TMP_DIR define.
---
 src/backend/backup/basebackup.c               |  3 +-
 .../replication/logical/reorderbuffer.c       | 17 ++++-----
 src/backend/replication/slot.c                | 36 +++++++++----------
 src/backend/utils/adt/genfile.c               |  3 +-
 src/bin/initdb/initdb.c                       |  2 +-
 src/bin/pg_rewind/filemap.c                   |  2 +-
 src/include/replication/slot.h                |  2 ++
 7 files changed, 35 insertions(+), 30 deletions(-)
  27.5% src/backend/replication/logical/
  60.8% src/backend/replication/
   3.9% src/backend/utils/adt/
   4.2% src/bin/
   3.3% src/

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 01b35e26bd..de16afac74 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -36,6 +36,7 @@
 #include "port.h"
 #include "postmaster/syslogger.h"
 #include "postmaster/walsummarizer.h"
+#include "replication/slot.h"
 #include "replication/walsender.h"
 #include "replication/walsender_private.h"
 #include "storage/bufpage.h"
@@ -161,7 +162,7 @@ static const char *const excludeDirContents[] =
 	 * even if the intention is to restore to another primary. See backup.sgml
 	 * for a more detailed description.
 	 */
-	"pg_replslot",
+	PG_REPLSLOT_DIR,
 
 	/* Contents removed on startup, see dsm_cleanup_for_mmap(). */
 	PG_DYNSHMEM_DIR,
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 00a8327e77..78166b6ab7 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -4571,9 +4571,9 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname)
 	DIR		   *spill_dir;
 	struct dirent *spill_de;
 	struct stat statbuf;
-	char		path[MAXPGPATH * 2 + 12];
+	char		path[MAXPGPATH * 2 + sizeof(PG_REPLSLOT_DIR)];
 
-	sprintf(path, "pg_replslot/%s", slotname);
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, slotname);
 
 	/* we're only handling directories here, skip if it's not ours */
 	if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode))
@@ -4586,14 +4586,14 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname)
 		if (strncmp(spill_de->d_name, "xid", 3) == 0)
 		{
 			snprintf(path, sizeof(path),
-					 "pg_replslot/%s/%s", slotname,
+					 "%s/%s/%s", PG_REPLSLOT_DIR, slotname,
 					 spill_de->d_name);
 
 			if (unlink(path) != 0)
 				ereport(ERROR,
 						(errcode_for_file_access(),
-						 errmsg("could not remove file \"%s\" during removal of pg_replslot/%s/xid*: %m",
-								path, slotname)));
+						 errmsg("could not remove file \"%s\" during removal of %s/%s/xid*: %m",
+								path, PG_REPLSLOT_DIR, slotname)));
 		}
 	}
 	FreeDir(spill_dir);
@@ -4612,7 +4612,8 @@ ReorderBufferSerializedPath(char *path, ReplicationSlot *slot, TransactionId xid
 
 	XLogSegNoOffsetToRecPtr(segno, 0, wal_segment_size, recptr);
 
-	snprintf(path, MAXPGPATH, "pg_replslot/%s/xid-%u-lsn-%X-%X.spill",
+	snprintf(path, MAXPGPATH, "%s/%s/xid-%u-lsn-%X-%X.spill",
+			 PG_REPLSLOT_DIR,
 			 NameStr(MyReplicationSlot->data.name),
 			 xid, LSN_FORMAT_ARGS(recptr));
 }
@@ -4627,8 +4628,8 @@ StartupReorderBuffer(void)
 	DIR		   *logical_dir;
 	struct dirent *logical_de;
 
-	logical_dir = AllocateDir("pg_replslot");
-	while ((logical_de = ReadDir(logical_dir, "pg_replslot")) != NULL)
+	logical_dir = AllocateDir(PG_REPLSLOT_DIR);
+	while ((logical_de = ReadDir(logical_dir, PG_REPLSLOT_DIR)) != NULL)
 	{
 		if (strcmp(logical_de->d_name, ".") == 0 ||
 			strcmp(logical_de->d_name, "..") == 0)
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index c290339af5..1299d92696 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -916,8 +916,8 @@ ReplicationSlotDropPtr(ReplicationSlot *slot)
 	LWLockAcquire(ReplicationSlotAllocationLock, LW_EXCLUSIVE);
 
 	/* Generate pathnames. */
-	sprintf(path, "pg_replslot/%s", NameStr(slot->data.name));
-	sprintf(tmppath, "pg_replslot/%s.tmp", NameStr(slot->data.name));
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(slot->data.name));
+	sprintf(tmppath, "%s/%s.tmp", PG_REPLSLOT_DIR, NameStr(slot->data.name));
 
 	/*
 	 * Rename the slot directory on disk, so that we'll no longer recognize
@@ -938,7 +938,7 @@ ReplicationSlotDropPtr(ReplicationSlot *slot)
 		 */
 		START_CRIT_SECTION();
 		fsync_fname(tmppath, true);
-		fsync_fname("pg_replslot", true);
+		fsync_fname(PG_REPLSLOT_DIR, true);
 		END_CRIT_SECTION();
 	}
 	else
@@ -1016,7 +1016,7 @@ ReplicationSlotSave(void)
 
 	Assert(MyReplicationSlot != NULL);
 
-	sprintf(path, "pg_replslot/%s", NameStr(MyReplicationSlot->data.name));
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(MyReplicationSlot->data.name));
 	SaveSlotToPath(MyReplicationSlot, path, ERROR);
 }
 
@@ -1881,7 +1881,7 @@ CheckPointReplicationSlots(bool is_shutdown)
 			continue;
 
 		/* save the slot to disk, locking is handled in SaveSlotToPath() */
-		sprintf(path, "pg_replslot/%s", NameStr(s->data.name));
+		sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(s->data.name));
 
 		/*
 		 * Slot's data is not flushed each time the confirmed_flush LSN is
@@ -1922,17 +1922,17 @@ StartupReplicationSlots(void)
 	elog(DEBUG1, "starting up replication slots");
 
 	/* restore all slots by iterating over all on-disk entries */
-	replication_dir = AllocateDir("pg_replslot");
-	while ((replication_de = ReadDir(replication_dir, "pg_replslot")) != NULL)
+	replication_dir = AllocateDir(PG_REPLSLOT_DIR);
+	while ((replication_de = ReadDir(replication_dir, PG_REPLSLOT_DIR)) != NULL)
 	{
-		char		path[MAXPGPATH + 12];
+		char		path[MAXPGPATH + sizeof(PG_REPLSLOT_DIR)];
 		PGFileType	de_type;
 
 		if (strcmp(replication_de->d_name, ".") == 0 ||
 			strcmp(replication_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(path, sizeof(path), "pg_replslot/%s", replication_de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_REPLSLOT_DIR, replication_de->d_name);
 		de_type = get_dirent_type(path, replication_de, false, DEBUG1);
 
 		/* we're only creating directories here, skip if it's not our's */
@@ -1949,7 +1949,7 @@ StartupReplicationSlots(void)
 								path)));
 				continue;
 			}
-			fsync_fname("pg_replslot", true);
+			fsync_fname(PG_REPLSLOT_DIR, true);
 			continue;
 		}
 
@@ -1987,8 +1987,8 @@ CreateSlotOnDisk(ReplicationSlot *slot)
 	 * takes out the lock, if we'd take the lock here, we'd deadlock.
 	 */
 
-	sprintf(path, "pg_replslot/%s", NameStr(slot->data.name));
-	sprintf(tmppath, "pg_replslot/%s.tmp", NameStr(slot->data.name));
+	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(slot->data.name));
+	sprintf(tmppath, "%s/%s.tmp", PG_REPLSLOT_DIR, NameStr(slot->data.name));
 
 	/*
 	 * It's just barely possible that some previous effort to create or drop a
@@ -2027,7 +2027,7 @@ CreateSlotOnDisk(ReplicationSlot *slot)
 	START_CRIT_SECTION();
 
 	fsync_fname(path, true);
-	fsync_fname("pg_replslot", true);
+	fsync_fname(PG_REPLSLOT_DIR, true);
 
 	END_CRIT_SECTION();
 }
@@ -2170,7 +2170,7 @@ SaveSlotToPath(ReplicationSlot *slot, const char *dir, int elevel)
 
 	fsync_fname(path, false);
 	fsync_fname(dir, true);
-	fsync_fname("pg_replslot", true);
+	fsync_fname(PG_REPLSLOT_DIR, true);
 
 	END_CRIT_SECTION();
 
@@ -2195,8 +2195,8 @@ RestoreSlotFromDisk(const char *name)
 {
 	ReplicationSlotOnDisk cp;
 	int			i;
-	char		slotdir[MAXPGPATH + 12];
-	char		path[MAXPGPATH + 22];
+	char		slotdir[MAXPGPATH + sizeof(PG_REPLSLOT_DIR)];
+	char		path[MAXPGPATH + sizeof(PG_REPLSLOT_DIR) + 10];
 	int			fd;
 	bool		restored = false;
 	int			readBytes;
@@ -2205,7 +2205,7 @@ RestoreSlotFromDisk(const char *name)
 	/* no need to lock here, no concurrent access allowed yet */
 
 	/* delete temp file if it exists */
-	sprintf(slotdir, "pg_replslot/%s", name);
+	sprintf(slotdir, "%s/%s", PG_REPLSLOT_DIR, name);
 	sprintf(path, "%s/state.tmp", slotdir);
 	if (unlink(path) < 0 && errno != ENOENT)
 		ereport(PANIC,
@@ -2332,7 +2332,7 @@ RestoreSlotFromDisk(const char *name)
 					(errmsg("could not remove directory \"%s\"",
 							slotdir)));
 		}
-		fsync_fname("pg_replslot", true);
+		fsync_fname(PG_REPLSLOT_DIR, true);
 		return;
 	}
 
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 0d82557417..d95683d041 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -728,6 +728,7 @@ pg_ls_replslotdir(PG_FUNCTION_ARGS)
 				 errmsg("replication slot \"%s\" does not exist",
 						slotname)));
 
-	snprintf(path, sizeof(path), "pg_replslot/%s", slotname);
+	snprintf(path, sizeof(path), "%s/%s", PG_REPLSLOT_DIR, slotname);
+
 	return pg_ls_dir_files(fcinfo, path, false);
 }
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index f00718a015..2ee55eced0 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -242,7 +242,7 @@ static const char *const subdirs[] = {
 	"pg_multixact/offsets",
 	"base",
 	"base/1",
-	"pg_replslot",
+	"pg_replslot",				/* defined as PG_REPLSLOT_DIR */
 	"pg_tblspc",
 	"pg_stat",
 	"pg_stat_tmp",
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 4458324c9d..00e644d988 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -97,7 +97,7 @@ static const char *const excludeDirContents[] =
 	 * even if the intention is to restore to another primary. See backup.sgml
 	 * for a more detailed description.
 	 */
-	"pg_replslot",
+	"pg_replslot",				/* defined as PG_REPLSLOT_DIR */
 
 	/* Contents removed on startup, see dsm_cleanup_for_mmap(). */
 	"pg_dynshmem",				/* defined as PG_DYNSHMEM_DIR */
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index c2ee149fd6..9665faac1c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -17,6 +17,8 @@
 #include "storage/spin.h"
 #include "replication/walreceiver.h"
 
+#define PG_REPLSLOT_DIR     "pg_replslot"
+
 /*
  * Behaviour of replication slots, upon release or crash.
  *
-- 
2.34.1

v3-0002-Define-REPLSLOT_DIR_ARGS.patchtext/x-diff; charset=us-asciiDownload

From 92299c4e7e6a53f53527498c9eff76c2595f82c2 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Tue, 20 Aug 2024 15:21:30 +0000
Subject: [PATCH v3 2/6] Define REPLSLOT_DIR_ARGS

Adding a new macro REPLSLOT_DIR_ARGS used when a slot name needs to be concatenated
to PG_REPLSLOT_DIR.
---
 src/backend/replication/logical/reorderbuffer.c |  9 ++++-----
 src/backend/replication/slot.c                  | 16 ++++++++--------
 src/backend/utils/adt/genfile.c                 |  2 +-
 src/include/replication/slot.h                  |  1 +
 4 files changed, 14 insertions(+), 14 deletions(-)
  23.5% src/backend/replication/logical/
  64.9% src/backend/replication/
   7.7% src/backend/utils/adt/
   3.7% src/include/replication/

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 78166b6ab7..e4c2851c57 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -4573,7 +4573,7 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname)
 	struct stat statbuf;
 	char		path[MAXPGPATH * 2 + sizeof(PG_REPLSLOT_DIR)];
 
-	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, slotname);
+	sprintf(path, "%s/%s", REPLSLOT_DIR_ARGS(slotname));
 
 	/* we're only handling directories here, skip if it's not ours */
 	if (lstat(path, &statbuf) == 0 && !S_ISDIR(statbuf.st_mode))
@@ -4586,14 +4586,14 @@ ReorderBufferCleanupSerializedTXNs(const char *slotname)
 		if (strncmp(spill_de->d_name, "xid", 3) == 0)
 		{
 			snprintf(path, sizeof(path),
-					 "%s/%s/%s", PG_REPLSLOT_DIR, slotname,
+					 "%s/%s/%s", REPLSLOT_DIR_ARGS(slotname),
 					 spill_de->d_name);
 
 			if (unlink(path) != 0)
 				ereport(ERROR,
 						(errcode_for_file_access(),
 						 errmsg("could not remove file \"%s\" during removal of %s/%s/xid*: %m",
-								path, PG_REPLSLOT_DIR, slotname)));
+								path, REPLSLOT_DIR_ARGS(slotname))));
 		}
 	}
 	FreeDir(spill_dir);
@@ -4613,8 +4613,7 @@ ReorderBufferSerializedPath(char *path, ReplicationSlot *slot, TransactionId xid
 	XLogSegNoOffsetToRecPtr(segno, 0, wal_segment_size, recptr);
 
 	snprintf(path, MAXPGPATH, "%s/%s/xid-%u-lsn-%X-%X.spill",
-			 PG_REPLSLOT_DIR,
-			 NameStr(MyReplicationSlot->data.name),
+			 REPLSLOT_DIR_ARGS(NameStr(MyReplicationSlot->data.name)),
 			 xid, LSN_FORMAT_ARGS(recptr));
 }
 
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 1299d92696..45225ca314 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -916,8 +916,8 @@ ReplicationSlotDropPtr(ReplicationSlot *slot)
 	LWLockAcquire(ReplicationSlotAllocationLock, LW_EXCLUSIVE);
 
 	/* Generate pathnames. */
-	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(slot->data.name));
-	sprintf(tmppath, "%s/%s.tmp", PG_REPLSLOT_DIR, NameStr(slot->data.name));
+	sprintf(path, "%s/%s", REPLSLOT_DIR_ARGS(NameStr(slot->data.name)));
+	sprintf(tmppath, "%s/%s.tmp", REPLSLOT_DIR_ARGS(NameStr(slot->data.name)));
 
 	/*
 	 * Rename the slot directory on disk, so that we'll no longer recognize
@@ -1016,7 +1016,7 @@ ReplicationSlotSave(void)
 
 	Assert(MyReplicationSlot != NULL);
 
-	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(MyReplicationSlot->data.name));
+	sprintf(path, "%s/%s", REPLSLOT_DIR_ARGS(NameStr(MyReplicationSlot->data.name)));
 	SaveSlotToPath(MyReplicationSlot, path, ERROR);
 }
 
@@ -1881,7 +1881,7 @@ CheckPointReplicationSlots(bool is_shutdown)
 			continue;
 
 		/* save the slot to disk, locking is handled in SaveSlotToPath() */
-		sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(s->data.name));
+		sprintf(path, "%s/%s", REPLSLOT_DIR_ARGS(NameStr(s->data.name)));
 
 		/*
 		 * Slot's data is not flushed each time the confirmed_flush LSN is
@@ -1932,7 +1932,7 @@ StartupReplicationSlots(void)
 			strcmp(replication_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(path, sizeof(path), "%s/%s", PG_REPLSLOT_DIR, replication_de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", REPLSLOT_DIR_ARGS(replication_de->d_name));
 		de_type = get_dirent_type(path, replication_de, false, DEBUG1);
 
 		/* we're only creating directories here, skip if it's not our's */
@@ -1987,8 +1987,8 @@ CreateSlotOnDisk(ReplicationSlot *slot)
 	 * takes out the lock, if we'd take the lock here, we'd deadlock.
 	 */
 
-	sprintf(path, "%s/%s", PG_REPLSLOT_DIR, NameStr(slot->data.name));
-	sprintf(tmppath, "%s/%s.tmp", PG_REPLSLOT_DIR, NameStr(slot->data.name));
+	sprintf(path, "%s/%s", REPLSLOT_DIR_ARGS(NameStr(slot->data.name)));
+	sprintf(tmppath, "%s/%s.tmp", REPLSLOT_DIR_ARGS(NameStr(slot->data.name)));
 
 	/*
 	 * It's just barely possible that some previous effort to create or drop a
@@ -2205,7 +2205,7 @@ RestoreSlotFromDisk(const char *name)
 	/* no need to lock here, no concurrent access allowed yet */
 
 	/* delete temp file if it exists */
-	sprintf(slotdir, "%s/%s", PG_REPLSLOT_DIR, name);
+	sprintf(slotdir, "%s/%s", REPLSLOT_DIR_ARGS(name));
 	sprintf(path, "%s/state.tmp", slotdir);
 	if (unlink(path) < 0 && errno != ENOENT)
 		ereport(PANIC,
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index d95683d041..82d68b0caa 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -728,7 +728,7 @@ pg_ls_replslotdir(PG_FUNCTION_ARGS)
 				 errmsg("replication slot \"%s\" does not exist",
 						slotname)));
 
-	snprintf(path, sizeof(path), "%s/%s", PG_REPLSLOT_DIR, slotname);
+	snprintf(path, sizeof(path), "%s/%s", REPLSLOT_DIR_ARGS(slotname));
 
 	return pg_ls_dir_files(fcinfo, path, false);
 }
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index 9665faac1c..150e3a228c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -18,6 +18,7 @@
 #include "replication/walreceiver.h"
 
 #define PG_REPLSLOT_DIR     "pg_replslot"
+#define REPLSLOT_DIR_ARGS(slotname) (PG_REPLSLOT_DIR), (slotname)
 
 /*
  * Behaviour of replication slots, upon release or crash.
-- 
2.34.1

v3-0003-Define-PG_LOGICAL_MAPPINGS_DIR.patchtext/x-diff; charset=us-asciiDownload

From 5ae8849d4bb81e9141a7bd7dd8fda39de18442b2 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 09:30:16 +0000
Subject: [PATCH v3 3/6] Define PG_LOGICAL_MAPPINGS_DIR

Replace most of the places where "pg_logical/mappings" is used in .c files with
a new PG_LOGICAL_MAPPINGS_DIR define. The places where it is not done is for
consistency with the existing PG_STAT_TMP_DIR define.
---
 src/backend/access/heap/rewriteheap.c          | 18 +++++++++---------
 .../replication/logical/reorderbuffer.c        |  6 +++---
 src/backend/utils/adt/genfile.c                |  2 +-
 src/include/replication/reorderbuffer.h        |  2 ++
 4 files changed, 15 insertions(+), 13 deletions(-)
  63.2% src/backend/access/heap/
  24.4% src/backend/replication/logical/
   8.5% src/backend/utils/adt/
   3.6% src/include/replication/

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 473f3aa9be..09ef220449 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -961,8 +961,8 @@ logical_rewrite_log_mapping(RewriteState state, TransactionId xid,
 			dboid = MyDatabaseId;
 
 		snprintf(path, MAXPGPATH,
-				 "pg_logical/mappings/" LOGICAL_REWRITE_FORMAT,
-				 dboid, relid,
+				 "%s/" LOGICAL_REWRITE_FORMAT,
+				 PG_LOGICAL_MAPPINGS_DIR, dboid, relid,
 				 LSN_FORMAT_ARGS(state->rs_begin_lsn),
 				 xid, GetCurrentTransactionId());
 
@@ -1081,8 +1081,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r)
 	xlrec = (xl_heap_rewrite_mapping *) XLogRecGetData(r);
 
 	snprintf(path, MAXPGPATH,
-			 "pg_logical/mappings/" LOGICAL_REWRITE_FORMAT,
-			 xlrec->mapped_db, xlrec->mapped_rel,
+			 "%s/" LOGICAL_REWRITE_FORMAT,
+			 PG_LOGICAL_MAPPINGS_DIR, xlrec->mapped_db, xlrec->mapped_rel,
 			 LSN_FORMAT_ARGS(xlrec->start_lsn),
 			 xlrec->mapped_xid, XLogRecGetXid(r));
 
@@ -1158,7 +1158,7 @@ CheckPointLogicalRewriteHeap(void)
 	XLogRecPtr	redo;
 	DIR		   *mappings_dir;
 	struct dirent *mapping_de;
-	char		path[MAXPGPATH + 20];
+	char		path[MAXPGPATH + sizeof(PG_LOGICAL_MAPPINGS_DIR)];
 
 	/*
 	 * We start of with a minimum of the last redo pointer. No new decoding
@@ -1173,8 +1173,8 @@ CheckPointLogicalRewriteHeap(void)
 	if (cutoff != InvalidXLogRecPtr && redo < cutoff)
 		cutoff = redo;
 
-	mappings_dir = AllocateDir("pg_logical/mappings");
-	while ((mapping_de = ReadDir(mappings_dir, "pg_logical/mappings")) != NULL)
+	mappings_dir = AllocateDir(PG_LOGICAL_MAPPINGS_DIR);
+	while ((mapping_de = ReadDir(mappings_dir, PG_LOGICAL_MAPPINGS_DIR)) != NULL)
 	{
 		Oid			dboid;
 		Oid			relid;
@@ -1189,7 +1189,7 @@ CheckPointLogicalRewriteHeap(void)
 			strcmp(mapping_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(path, sizeof(path), "pg_logical/mappings/%s", mapping_de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_LOGICAL_MAPPINGS_DIR, mapping_de->d_name);
 		de_type = get_dirent_type(path, mapping_de, false, DEBUG1);
 
 		if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG)
@@ -1249,5 +1249,5 @@ CheckPointLogicalRewriteHeap(void)
 	FreeDir(mappings_dir);
 
 	/* persist directory entries to disk */
-	fsync_fname("pg_logical/mappings", true);
+	fsync_fname(PG_LOGICAL_MAPPINGS_DIR, true);
 }
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index e4c2851c57..75fb6ef61a 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -5056,7 +5056,7 @@ ApplyLogicalMappingFile(HTAB *tuplecid_data, Oid relid, const char *fname)
 	int			readBytes;
 	LogicalRewriteMappingData map;
 
-	sprintf(path, "pg_logical/mappings/%s", fname);
+	sprintf(path, "%s/%s", PG_LOGICAL_MAPPINGS_DIR, fname);
 	fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
 	if (fd < 0)
 		ereport(ERROR,
@@ -5172,8 +5172,8 @@ UpdateLogicalMappings(HTAB *tuplecid_data, Oid relid, Snapshot snapshot)
 	ListCell   *file;
 	Oid			dboid = IsSharedRelation(relid) ? InvalidOid : MyDatabaseId;
 
-	mapping_dir = AllocateDir("pg_logical/mappings");
-	while ((mapping_de = ReadDir(mapping_dir, "pg_logical/mappings")) != NULL)
+	mapping_dir = AllocateDir(PG_LOGICAL_MAPPINGS_DIR);
+	while ((mapping_de = ReadDir(mapping_dir, PG_LOGICAL_MAPPINGS_DIR)) != NULL)
 	{
 		Oid			f_dboid;
 		Oid			f_relid;
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 82d68b0caa..625b966b6a 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -704,7 +704,7 @@ pg_ls_logicalsnapdir(PG_FUNCTION_ARGS)
 Datum
 pg_ls_logicalmapdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, "pg_logical/mappings", false);
+	return pg_ls_dir_files(fcinfo, PG_LOGICAL_MAPPINGS_DIR, false);
 }
 
 /*
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index 851a001c8b..bf3b8010f6 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -18,6 +18,8 @@
 #include "utils/snapshot.h"
 #include "utils/timestamp.h"
 
+#define PG_LOGICAL_MAPPINGS_DIR "pg_logical/mappings"
+
 /* GUC variables */
 extern PGDLLIMPORT int logical_decoding_work_mem;
 extern PGDLLIMPORT int debug_logical_replication_streaming;
-- 
2.34.1

v3-0004-Define-PG_LOGICAL_SNAPSHOTS_DIR.patchtext/x-diff; charset=us-asciiDownload

From d2781a8eff5f0311c7ac6bcb617b92c3f7989d3e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 09:42:40 +0000
Subject: [PATCH v3 4/6] Define PG_LOGICAL_SNAPSHOTS_DIR

Replace most of the places where "pg_logical/snapshots" is used in .c files with
a new PG_LOGICAL_SNAPSHOTS_DIR define. The places where it is not done is for
consistency with the existing PG_STAT_TMP_DIR define.
---
 src/backend/replication/logical/snapbuild.c | 28 ++++++++++++---------
 src/backend/utils/adt/genfile.c             |  2 +-
 src/include/replication/reorderbuffer.h     |  1 +
 3 files changed, 18 insertions(+), 13 deletions(-)
  87.6% src/backend/replication/logical/
   8.6% src/backend/utils/adt/
   3.7% src/include/replication/

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index ae676145e6..0450f94ba8 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1654,7 +1654,8 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 	 * unless the user used pg_resetwal or similar. If a user did so, there's
 	 * no hope continuing to decode anyway.
 	 */
-	sprintf(path, "pg_logical/snapshots/%X-%X.snap",
+	sprintf(path, "%s/%X-%X.snap",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn));
 
 	/*
@@ -1681,7 +1682,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 		 * be safely on disk.
 		 */
 		fsync_fname(path, false);
-		fsync_fname("pg_logical/snapshots", true);
+		fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 		builder->last_serialized_snapshot = lsn;
 		goto out;
@@ -1697,7 +1698,8 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 	elog(DEBUG1, "serializing snapshot to %s", path);
 
 	/* to make sure only we will write to this tempfile, include pid */
-	sprintf(tmppath, "pg_logical/snapshots/%X-%X.snap.%d.tmp",
+	sprintf(tmppath, "%s/%X-%X.snap.%d.tmp",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn), MyProcPid);
 
 	/*
@@ -1818,7 +1820,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 				(errcode_for_file_access(),
 				 errmsg("could not close file \"%s\": %m", tmppath)));
 
-	fsync_fname("pg_logical/snapshots", true);
+	fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 	/*
 	 * We may overwrite the work from some other backend, but that's ok, our
@@ -1834,7 +1836,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 
 	/* make sure we persist */
 	fsync_fname(path, false);
-	fsync_fname("pg_logical/snapshots", true);
+	fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 	/*
 	 * Now there's no way we can lose the dumped state anymore, remember this
@@ -1871,7 +1873,8 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
 	if (builder->state == SNAPBUILD_CONSISTENT)
 		return false;
 
-	sprintf(path, "pg_logical/snapshots/%X-%X.snap",
+	sprintf(path, "%s/%X-%X.snap",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn));
 
 	fd = OpenTransientFile(path, O_RDONLY | PG_BINARY);
@@ -1892,7 +1895,7 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
 	 * ----
 	 */
 	fsync_fname(path, false);
-	fsync_fname("pg_logical/snapshots", true);
+	fsync_fname(PG_LOGICAL_SNAPSHOTS_DIR, true);
 
 
 	/* read statically sized portion of snapshot */
@@ -2074,7 +2077,7 @@ CheckPointSnapBuild(void)
 	XLogRecPtr	redo;
 	DIR		   *snap_dir;
 	struct dirent *snap_de;
-	char		path[MAXPGPATH + 21];
+	char		path[MAXPGPATH + sizeof(PG_LOGICAL_SNAPSHOTS_DIR)];
 
 	/*
 	 * We start off with a minimum of the last redo pointer. No new
@@ -2090,8 +2093,8 @@ CheckPointSnapBuild(void)
 	if (redo < cutoff)
 		cutoff = redo;
 
-	snap_dir = AllocateDir("pg_logical/snapshots");
-	while ((snap_de = ReadDir(snap_dir, "pg_logical/snapshots")) != NULL)
+	snap_dir = AllocateDir(PG_LOGICAL_SNAPSHOTS_DIR);
+	while ((snap_de = ReadDir(snap_dir, PG_LOGICAL_SNAPSHOTS_DIR)) != NULL)
 	{
 		uint32		hi;
 		uint32		lo;
@@ -2102,7 +2105,7 @@ CheckPointSnapBuild(void)
 			strcmp(snap_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(path, sizeof(path), "pg_logical/snapshots/%s", snap_de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_LOGICAL_SNAPSHOTS_DIR, snap_de->d_name);
 		de_type = get_dirent_type(path, snap_de, false, DEBUG1);
 
 		if (de_type != PGFILETYPE_ERROR && de_type != PGFILETYPE_REG)
@@ -2162,7 +2165,8 @@ SnapBuildSnapshotExists(XLogRecPtr lsn)
 	int			ret;
 	struct stat stat_buf;
 
-	sprintf(path, "pg_logical/snapshots/%X-%X.snap",
+	sprintf(path, "%s/%X-%X.snap",
+			PG_LOGICAL_SNAPSHOTS_DIR,
 			LSN_FORMAT_ARGS(lsn));
 
 	ret = stat(path, &stat_buf);
diff --git a/src/backend/utils/adt/genfile.c b/src/backend/utils/adt/genfile.c
index 625b966b6a..a480e35693 100644
--- a/src/backend/utils/adt/genfile.c
+++ b/src/backend/utils/adt/genfile.c
@@ -695,7 +695,7 @@ pg_ls_archive_statusdir(PG_FUNCTION_ARGS)
 Datum
 pg_ls_logicalsnapdir(PG_FUNCTION_ARGS)
 {
-	return pg_ls_dir_files(fcinfo, "pg_logical/snapshots", false);
+	return pg_ls_dir_files(fcinfo, PG_LOGICAL_SNAPSHOTS_DIR, false);
 }
 
 /*
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index bf3b8010f6..60a30763c9 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -19,6 +19,7 @@
 #include "utils/timestamp.h"
 
 #define PG_LOGICAL_MAPPINGS_DIR "pg_logical/mappings"
+#define PG_LOGICAL_SNAPSHOTS_DIR "pg_logical/snapshots"
 
 /* GUC variables */
 extern PGDLLIMPORT int logical_decoding_work_mem;
-- 
2.34.1

v3-0005-Define-PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR.patchtext/x-diff; charset=us-asciiDownload

From a67f92196998e1f27bcdedb955546e137ee4e8b6 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 09:53:03 +0000
Subject: [PATCH v3 5/6] Define PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR

Replace the places where "pg_logical/replorigin_checkpoint" is used in .c files
with a new PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR define.
---
 src/backend/replication/logical/origin.c | 6 +++---
 src/include/replication/origin.h         | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)
  67.1% src/backend/replication/logical/
  32.8% src/include/replication/

diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 419e4814f0..a4aaefa97c 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -572,8 +572,8 @@ ReplicationOriginShmemInit(void)
 void
 CheckPointReplicationOrigin(void)
 {
-	const char *tmppath = "pg_logical/replorigin_checkpoint.tmp";
-	const char *path = "pg_logical/replorigin_checkpoint";
+	const char *tmppath = PG_LOGICAL_REPLORIGIN_CHECKPOINT_TMP_DIR;
+	const char *path = PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR;
 	int			tmpfd;
 	int			i;
 	uint32		magic = REPLICATION_STATE_MAGIC;
@@ -698,7 +698,7 @@ CheckPointReplicationOrigin(void)
 void
 StartupReplicationOrigin(void)
 {
-	const char *path = "pg_logical/replorigin_checkpoint";
+	const char *path = PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR;
 	int			fd;
 	int			readBytes;
 	uint32		magic = REPLICATION_STATE_MAGIC;
diff --git a/src/include/replication/origin.h b/src/include/replication/origin.h
index 7189ba9e76..1cb44be58d 100644
--- a/src/include/replication/origin.h
+++ b/src/include/replication/origin.h
@@ -15,6 +15,9 @@
 #include "access/xlogreader.h"
 #include "catalog/pg_replication_origin.h"
 
+#define PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR "pg_logical/replorigin_checkpoint"
+#define PG_LOGICAL_REPLORIGIN_CHECKPOINT_TMP_DIR PG_LOGICAL_REPLORIGIN_CHECKPOINT_DIR ".tmp"
+
 typedef struct xl_replorigin_set
 {
 	XLogRecPtr	remote_lsn;
-- 
2.34.1

v3-0006-Define-PG_TBLSPC_DIR.patchtext/x-diff; charset=us-asciiDownload

From a30ab33122eb972b18c5f33dcf4b7feb7df2c433 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 07:40:10 +0000
Subject: [PATCH v3 6/6] Define PG_TBLSPC_DIR

Replace most of the places where "pg_tblspc" is used in .c files with a new
PG_TBLSPC_DIR define. The places where it is not done is for consistency with
the existing PG_STAT_TMP_DIR define.
---
 src/backend/access/transam/xlog.c           | 12 ++++-----
 src/backend/access/transam/xlogrecovery.c   | 14 +++++-----
 src/backend/backup/backup_manifest.c        |  3 ++-
 src/backend/backup/basebackup.c             |  6 ++---
 src/backend/commands/dbcommands.c           |  2 +-
 src/backend/commands/tablespace.c           |  4 +--
 src/backend/storage/file/fd.c               | 29 +++++++++++----------
 src/backend/storage/file/reinit.c           | 10 +++----
 src/backend/utils/adt/dbsize.c              |  8 +++---
 src/backend/utils/adt/misc.c                |  4 +--
 src/backend/utils/cache/relcache.c          |  4 +--
 src/bin/pg_checksums/pg_checksums.c         |  6 ++---
 src/bin/pg_combinebackup/pg_combinebackup.c | 18 ++++++-------
 src/bin/pg_rewind/file_ops.c                |  2 +-
 src/bin/pg_upgrade/exec.c                   |  2 +-
 src/common/file_utils.c                     |  3 ++-
 src/common/relpath.c                        | 20 +++++++-------
 src/fe_utils/astreamer_file.c               |  7 +++--
 src/include/common/relpath.h                |  8 ++++++
 19 files changed, 88 insertions(+), 74 deletions(-)
  14.9% src/backend/access/transam/
   6.4% src/backend/backup/
   4.3% src/backend/commands/
  25.8% src/backend/storage/file/
   8.5% src/backend/utils/adt/
   4.1% src/bin/pg_checksums/
  11.2% src/bin/pg_combinebackup/
  12.8% src/common/
   3.5% src/fe_utils/
   3.0% src/include/common/
   5.0% src/

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ee0fb0e28f..4e06d86196 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8944,10 +8944,10 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 		datadirpathlen = strlen(DataDir);
 
 		/* Collect information about all tablespaces */
-		tblspcdir = AllocateDir("pg_tblspc");
-		while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL)
+		tblspcdir = AllocateDir(PG_TBLSPC_DIR);
+		while ((de = ReadDir(tblspcdir, PG_TBLSPC_DIR)) != NULL)
 		{
-			char		fullpath[MAXPGPATH + 10];
+			char		fullpath[MAXPGPATH + sizeof(PG_TBLSPC_DIR)];
 			char		linkpath[MAXPGPATH];
 			char	   *relpath = NULL;
 			char	   *s;
@@ -8970,7 +8970,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			if (*badp != '\0' || errno == EINVAL || errno == ERANGE)
 				continue;
 
-			snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name);
+			snprintf(fullpath, sizeof(fullpath), "%s/%s", PG_TBLSPC_DIR, de->d_name);
 
 			de_type = get_dirent_type(fullpath, de, false, ERROR);
 
@@ -9031,8 +9031,8 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 				 * 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);
+				snprintf(linkpath, sizeof(linkpath), "%s/%s",
+						 PG_TBLSPC_DIR, de->d_name);
 				relpath = pstrdup(linkpath);
 			}
 			else
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ad817fbca6..5d2755ef79 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -677,7 +677,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
 				tablespaceinfo *ti = lfirst(lc);
 				char	   *linkloc;
 
-				linkloc = psprintf("pg_tblspc/%u", ti->oid);
+				linkloc = psprintf("%s/%u", PG_TBLSPC_DIR, ti->oid);
 
 				/*
 				 * Remove the existing symlink if any and Create the symlink
@@ -2157,23 +2157,23 @@ CheckTablespaceDirectory(void)
 	DIR		   *dir;
 	struct dirent *de;
 
-	dir = AllocateDir("pg_tblspc");
-	while ((de = ReadDir(dir, "pg_tblspc")) != NULL)
+	dir = AllocateDir(PG_TBLSPC_DIR);
+	while ((de = ReadDir(dir, PG_TBLSPC_DIR)) != NULL)
 	{
-		char		path[MAXPGPATH + 10];
+		char		path[MAXPGPATH + sizeof(PG_TBLSPC_DIR)];
 
 		/* Skip entries of non-oid names */
 		if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
 			continue;
 
-		snprintf(path, sizeof(path), "pg_tblspc/%s", de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_TBLSPC_DIR, de->d_name);
 
 		if (get_dirent_type(path, de, false, ERROR) != PGFILETYPE_LNK)
 			ereport(allow_in_place_tablespaces ? WARNING : PANIC,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg("unexpected directory entry \"%s\" found in %s",
-							de->d_name, "pg_tblspc/"),
-					 errdetail("All directory entries in pg_tblspc/ should be symbolic links."),
+							de->d_name, PG_TBLSPC_DIR),
+					 errdetail("All directory entries in %s/ should be symbolic links.", PG_TBLSPC_DIR),
 					 errhint("Remove those directories, or set \"allow_in_place_tablespaces\" to ON transiently to let recovery complete.")));
 	}
 }
diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c
index 4357cfa31d..a2e2f86332 100644
--- a/src/backend/backup/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -16,6 +16,7 @@
 #include "access/xlog.h"
 #include "backup/backup_manifest.h"
 #include "backup/basebackup_sink.h"
+#include "common/relpath.h"
 #include "mb/pg_wchar.h"
 #include "utils/builtins.h"
 #include "utils/json.h"
@@ -117,7 +118,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, Oid spcoid,
 	 */
 	if (OidIsValid(spcoid))
 	{
-		snprintf(pathbuf, sizeof(pathbuf), "pg_tblspc/%u/%s", spcoid,
+		snprintf(pathbuf, sizeof(pathbuf), "%s/%u/%s", PG_TBLSPC_DIR, spcoid,
 				 pathname);
 		pathname = pathbuf;
 	}
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index de16afac74..15805ce90e 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -1406,7 +1406,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 		}
 
 		/* Allow symbolic links in pg_tblspc only */
-		if (strcmp(path, "./pg_tblspc") == 0 && S_ISLNK(statbuf.st_mode))
+		if (strcmp(path, RELATIVE_PG_TBLSPC_DIR) == 0 && S_ISLNK(statbuf.st_mode))
 		{
 			char		linkpath[MAXPGPATH];
 			int			rllen;
@@ -1464,7 +1464,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 			/*
 			 * skip sending directories inside pg_tblspc, if not required.
 			 */
-			if (strcmp(pathbuf, "./pg_tblspc") == 0 && !sendtblspclinks)
+			if (strcmp(pathbuf, RELATIVE_PG_TBLSPC_DIR) == 0 && !sendtblspclinks)
 				skip_this_dir = true;
 
 			if (!skip_this_dir)
@@ -1488,7 +1488,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 				if (OidIsValid(spcoid))
 				{
 					relspcoid = spcoid;
-					lookup_path = psprintf("pg_tblspc/%u/%s", spcoid,
+					lookup_path = psprintf("%s/%u/%s", PG_TBLSPC_DIR, spcoid,
 										   tarfilename);
 				}
 				else
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index d00ae40e19..8be435a79e 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -3257,7 +3257,7 @@ recovery_create_dbdir(char *path, bool only_tblspc)
 	if (stat(path, &st) == 0)
 		return;
 
-	if (only_tblspc && strstr(path, "pg_tblspc/") == NULL)
+	if (only_tblspc && strstr(path, PG_TBLSPC_DIR_SLASH) == NULL)
 		elog(PANIC, "requested to created invalid directory: %s", path);
 
 	if (reachedConsistency && !allow_in_place_tablespaces)
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 113b480731..00c1ed19fd 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -576,7 +576,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
 	struct stat st;
 	bool		in_place;
 
-	linkloc = psprintf("pg_tblspc/%u", tablespaceoid);
+	linkloc = psprintf("%s/%u", PG_TBLSPC_DIR, tablespaceoid);
 
 	/*
 	 * If we're asked to make an 'in place' tablespace, create the directory
@@ -692,7 +692,7 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo)
 	char	   *subfile;
 	struct stat st;
 
-	linkloc_with_version_dir = psprintf("pg_tblspc/%u/%s", tablespaceoid,
+	linkloc_with_version_dir = psprintf("%s/%u/%s", PG_TBLSPC_DIR, tablespaceoid,
 										TABLESPACE_VERSION_DIRECTORY);
 
 	/*
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 3944321ff3..db24343bf9 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1790,8 +1790,8 @@ TempTablespacePath(char *path, Oid tablespace)
 	else
 	{
 		/* All other tablespaces are accessed via symlinks */
-		snprintf(path, MAXPGPATH, "pg_tblspc/%u/%s/%s",
-				 tablespace, TABLESPACE_VERSION_DIRECTORY,
+		snprintf(path, MAXPGPATH, "%s/%u/%s/%s",
+				 PG_TBLSPC_DIR, tablespace, TABLESPACE_VERSION_DIRECTORY,
 				 PG_TEMP_FILES_DIR);
 	}
 }
@@ -3273,7 +3273,7 @@ CleanupTempFiles(bool isCommit, bool isProcExit)
 void
 RemovePgTempFiles(void)
 {
-	char		temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY) + sizeof(PG_TEMP_FILES_DIR)];
+	char		temp_path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY) + sizeof(PG_TEMP_FILES_DIR)];
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 
@@ -3287,20 +3287,21 @@ RemovePgTempFiles(void)
 	/*
 	 * Cycle through temp directories for all non-default tablespaces.
 	 */
-	spc_dir = AllocateDir("pg_tblspc");
+	spc_dir = AllocateDir(PG_TBLSPC_DIR);
 
-	while ((spc_de = ReadDirExtended(spc_dir, "pg_tblspc", LOG)) != NULL)
+	while ((spc_de = ReadDirExtended(spc_dir, PG_TBLSPC_DIR, LOG)) != NULL)
 	{
 		if (strcmp(spc_de->d_name, ".") == 0 ||
 			strcmp(spc_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY, PG_TEMP_FILES_DIR);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY,
+				 PG_TEMP_FILES_DIR);
 		RemovePgTempFilesInDir(temp_path, true, false);
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
 		RemovePgTempRelationFiles(temp_path);
 	}
 
@@ -3587,15 +3588,15 @@ SyncDataDirectory(void)
 		/* Sync the top level pgdata directory. */
 		do_syncfs(".");
 		/* If any tablespaces are configured, sync each of those. */
-		dir = AllocateDir("pg_tblspc");
-		while ((de = ReadDirExtended(dir, "pg_tblspc", LOG)))
+		dir = AllocateDir(PG_TBLSPC_DIR);
+		while ((de = ReadDirExtended(dir, PG_TBLSPC_DIR, LOG)))
 		{
 			char		path[MAXPGPATH];
 
 			if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
 				continue;
 
-			snprintf(path, MAXPGPATH, "pg_tblspc/%s", de->d_name);
+			snprintf(path, MAXPGPATH, "%s/%s", PG_TBLSPC_DIR, de->d_name);
 			do_syncfs(path);
 		}
 		FreeDir(dir);
@@ -3618,7 +3619,7 @@ SyncDataDirectory(void)
 	walkdir(".", pre_sync_fname, false, DEBUG1);
 	if (xlog_is_symlink)
 		walkdir("pg_wal", pre_sync_fname, false, DEBUG1);
-	walkdir("pg_tblspc", pre_sync_fname, true, DEBUG1);
+	walkdir(PG_TBLSPC_DIR, pre_sync_fname, true, DEBUG1);
 #endif
 
 	/* Prepare to report progress syncing the data directory via fsync. */
@@ -3636,7 +3637,7 @@ SyncDataDirectory(void)
 	walkdir(".", datadir_fsync_fname, false, LOG);
 	if (xlog_is_symlink)
 		walkdir("pg_wal", datadir_fsync_fname, false, LOG);
-	walkdir("pg_tblspc", datadir_fsync_fname, true, LOG);
+	walkdir(PG_TBLSPC_DIR, datadir_fsync_fname, true, LOG);
 }
 
 /*
diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c
index f1cd1a38d9..01e267abf9 100644
--- a/src/backend/storage/file/reinit.c
+++ b/src/backend/storage/file/reinit.c
@@ -46,7 +46,7 @@ typedef struct
 void
 ResetUnloggedRelations(int op)
 {
-	char		temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)];
+	char		temp_path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY)];
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 	MemoryContext tmpctx,
@@ -77,16 +77,16 @@ ResetUnloggedRelations(int op)
 	/*
 	 * Cycle through directories for all non-default tablespaces.
 	 */
-	spc_dir = AllocateDir("pg_tblspc");
+	spc_dir = AllocateDir(PG_TBLSPC_DIR);
 
-	while ((spc_de = ReadDir(spc_dir, "pg_tblspc")) != NULL)
+	while ((spc_de = ReadDir(spc_dir, PG_TBLSPC_DIR)) != NULL)
 	{
 		if (strcmp(spc_de->d_name, ".") == 0 ||
 			strcmp(spc_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
 		ResetUnloggedRelationsInTablespaceDir(temp_path, op);
 	}
 
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index b2d9cc2792..e63e99c141 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -143,7 +143,7 @@ calculate_database_size(Oid dbOid)
 	totalsize = db_dir_size(pathname);
 
 	/* Scan the non-default tablespaces */
-	snprintf(dirpath, MAXPGPATH, "pg_tblspc");
+	snprintf(dirpath, MAXPGPATH, PG_TBLSPC_DIR);
 	dirdesc = AllocateDir(dirpath);
 
 	while ((direntry = ReadDir(dirdesc, dirpath)) != NULL)
@@ -154,8 +154,8 @@ calculate_database_size(Oid dbOid)
 			strcmp(direntry->d_name, "..") == 0)
 			continue;
 
-		snprintf(pathname, sizeof(pathname), "pg_tblspc/%s/%s/%u",
-				 direntry->d_name, TABLESPACE_VERSION_DIRECTORY, dbOid);
+		snprintf(pathname, sizeof(pathname), "%s/%s/%s/%u",
+				 PG_TBLSPC_DIR, direntry->d_name, TABLESPACE_VERSION_DIRECTORY, dbOid);
 		totalsize += db_dir_size(pathname);
 	}
 
@@ -227,7 +227,7 @@ calculate_tablespace_size(Oid tblspcOid)
 	else if (tblspcOid == GLOBALTABLESPACE_OID)
 		snprintf(tblspcPath, MAXPGPATH, "global");
 	else
-		snprintf(tblspcPath, MAXPGPATH, "pg_tblspc/%u/%s", tblspcOid,
+		snprintf(tblspcPath, MAXPGPATH, "%s/%u/%s", PG_TBLSPC_DIR, tblspcOid,
 				 TABLESPACE_VERSION_DIRECTORY);
 
 	dirdesc = AllocateDir(tblspcPath);
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 0e6c45807a..1aa8bbb306 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -242,7 +242,7 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
 	if (tablespaceOid == DEFAULTTABLESPACE_OID)
 		location = "base";
 	else
-		location = psprintf("pg_tblspc/%u/%s", tablespaceOid,
+		location = psprintf("%s/%u/%s", PG_TBLSPC_DIR, tablespaceOid,
 							TABLESPACE_VERSION_DIRECTORY);
 
 	dirdesc = AllocateDir(location);
@@ -325,7 +325,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
 	 * Find the location of the tablespace by reading the symbolic link that
 	 * is in pg_tblspc/<oid>.
 	 */
-	snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%u", tablespaceOid);
+	snprintf(sourcepath, sizeof(sourcepath), "%s/%u", PG_TBLSPC_DIR, tablespaceOid);
 
 	/*
 	 * Before reading the link, check if the source path is a link or a
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 66ed24e401..63efc55f09 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -6800,10 +6800,10 @@ RelationCacheInitFilePostInvalidate(void)
 void
 RelationCacheInitFileRemove(void)
 {
-	const char *tblspcdir = "pg_tblspc";
+	const char *tblspcdir = PG_TBLSPC_DIR;
 	DIR		   *dir;
 	struct dirent *de;
-	char		path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)];
+	char		path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY)];
 
 	snprintf(path, sizeof(path), "global/%s",
 			 RELCACHE_INIT_FILENAME);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index b5bb0e7887..68a68eb0fa 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -388,7 +388,7 @@ scan_directory(const char *basedir, const char *subdir, bool sizeonly)
 			 * is valid, resolving the linked locations and dive into them
 			 * directly.
 			 */
-			if (strncmp("pg_tblspc", subdir, strlen("pg_tblspc")) == 0)
+			if (strncmp(PG_TBLSPC_DIR, subdir, strlen(PG_TBLSPC_DIR)) == 0)
 			{
 				char		tblspc_path[MAXPGPATH];
 				struct stat tblspc_st;
@@ -593,12 +593,12 @@ main(int argc, char *argv[])
 		{
 			total_size = scan_directory(DataDir, "global", true);
 			total_size += scan_directory(DataDir, "base", true);
-			total_size += scan_directory(DataDir, "pg_tblspc", true);
+			total_size += scan_directory(DataDir, PG_TBLSPC_DIR, true);
 		}
 
 		(void) scan_directory(DataDir, "global", false);
 		(void) scan_directory(DataDir, "base", false);
-		(void) scan_directory(DataDir, "pg_tblspc", false);
+		(void) scan_directory(DataDir, PG_TBLSPC_DIR, false);
 
 		if (showprogress)
 			progress_report(true);
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 9ded5a2140..61a97dd206 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -373,7 +373,7 @@ main(int argc, char *argv[])
 		{
 			char		linkpath[MAXPGPATH];
 
-			snprintf(linkpath, MAXPGPATH, "%s/pg_tblspc/%u", opt.output,
+			snprintf(linkpath, MAXPGPATH, "%s/%s/%u", opt.output, PG_TBLSPC_DIR,
 					 ts->oid);
 
 			if (opt.dry_run)
@@ -867,12 +867,12 @@ process_directory_recursively(Oid tsoid,
 		is_incremental_dir = true;
 	else if (relative_path != NULL)
 	{
-		is_pg_tblspc = strcmp(relative_path, "pg_tblspc") == 0;
+		is_pg_tblspc = strcmp(relative_path, PG_TBLSPC_DIR) == 0;
 		is_pg_wal = (strcmp(relative_path, "pg_wal") == 0 ||
 					 strncmp(relative_path, "pg_wal/", 7) == 0);
 		is_incremental_dir = strncmp(relative_path, "base/", 5) == 0 ||
 			strcmp(relative_path, "global") == 0 ||
-			strncmp(relative_path, "pg_tblspc/", 10) == 0;
+			strncmp(relative_path, PG_TBLSPC_DIR_SLASH, 10) == 0;
 	}
 
 	/*
@@ -895,7 +895,7 @@ process_directory_recursively(Oid tsoid,
 		strlcpy(ifulldir, input_directory, MAXPGPATH);
 		strlcpy(ofulldir, output_directory, MAXPGPATH);
 		if (OidIsValid(tsoid))
-			snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/", tsoid);
+			snprintf(manifest_prefix, MAXPGPATH, "%s/%u/", PG_TBLSPC_DIR, tsoid);
 		else
 			manifest_prefix[0] = '\0';
 	}
@@ -906,8 +906,8 @@ process_directory_recursively(Oid tsoid,
 		snprintf(ofulldir, MAXPGPATH, "%s/%s", output_directory,
 				 relative_path);
 		if (OidIsValid(tsoid))
-			snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/%s/",
-					 tsoid, relative_path);
+			snprintf(manifest_prefix, MAXPGPATH, "%s/%u/%s/",
+					 PG_TBLSPC_DIR, tsoid, relative_path);
 		else
 			snprintf(manifest_prefix, MAXPGPATH, "%s/", relative_path);
 	}
@@ -1249,7 +1249,7 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 	struct dirent *de;
 	cb_tablespace *tslist = NULL;
 
-	snprintf(pg_tblspc, MAXPGPATH, "%s/pg_tblspc", pathname);
+	snprintf(pg_tblspc, MAXPGPATH, "%s/%s", pathname, PG_TBLSPC_DIR);
 	pg_log_debug("scanning \"%s\"", pg_tblspc);
 
 	if ((dir = opendir(pg_tblspc)) == NULL)
@@ -1344,8 +1344,8 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 			 * we just record the paths within the data directories.
 			 */
 			snprintf(ts->old_dir, MAXPGPATH, "%s/%s", pg_tblspc, de->d_name);
-			snprintf(ts->new_dir, MAXPGPATH, "%s/pg_tblspc/%s", opt->output,
-					 de->d_name);
+			snprintf(ts->new_dir, MAXPGPATH, "%s/%s/%s", opt->output,
+					 PG_TBLSPC_DIR, de->d_name);
 			ts->in_place = true;
 		}
 
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index d93580bb41..67a86bb4c5 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -452,7 +452,7 @@ recurse_dir(const char *datadir, const char *parentpath,
 			 * to process all the tablespaces.  We also follow a symlink if
 			 * it's for pg_wal.  Symlinks elsewhere are ignored.
 			 */
-			if ((parentpath && strcmp(parentpath, "pg_tblspc") == 0) ||
+			if ((parentpath && strcmp(parentpath, PG_TBLSPC_DIR) == 0) ||
 				strcmp(path, "pg_wal") == 0)
 				recurse_dir(datadir, path, callback);
 		}
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 058530ab3e..78db321ace 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -350,7 +350,7 @@ check_data_dir(ClusterInfo *cluster)
 	check_single_dir(pg_data, "global");
 	check_single_dir(pg_data, "pg_multixact");
 	check_single_dir(pg_data, "pg_subtrans");
-	check_single_dir(pg_data, "pg_tblspc");
+	check_single_dir(pg_data, PG_TBLSPC_DIR);
 	check_single_dir(pg_data, "pg_twophase");
 
 	/* pg_xlog has been renamed to pg_wal in v10 */
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 6bac537a1e..761873d519 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 
 #include "common/file_utils.h"
+#include "common/relpath.h"
 #ifdef FRONTEND
 #include "common/logging.h"
 #endif
@@ -105,7 +106,7 @@ sync_pgdata(const char *pg_data,
 	/* handle renaming of pg_xlog to pg_wal in post-10 clusters */
 	snprintf(pg_wal, MAXPGPATH, "%s/%s", pg_data,
 			 serverVersion < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal");
-	snprintf(pg_tblspc, MAXPGPATH, "%s/pg_tblspc", pg_data);
+	snprintf(pg_tblspc, MAXPGPATH, "%s/%s", pg_data, PG_TBLSPC_DIR);
 
 	/*
 	 * If pg_wal is a symlink, we'll need to recurse into it separately,
diff --git a/src/common/relpath.c b/src/common/relpath.c
index f54c36ef7a..426a0114f8 100644
--- a/src/common/relpath.c
+++ b/src/common/relpath.c
@@ -123,8 +123,8 @@ GetDatabasePath(Oid dbOid, Oid spcOid)
 	else
 	{
 		/* All other tablespaces are accessed via symlinks */
-		return psprintf("pg_tblspc/%u/%s/%u",
-						spcOid, TABLESPACE_VERSION_DIRECTORY, dbOid);
+		return psprintf("%s/%u/%s/%u",
+						PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY, dbOid);
 	}
 }
 
@@ -184,25 +184,25 @@ GetRelationPath(Oid dbOid, Oid spcOid, RelFileNumber relNumber,
 		if (procNumber == INVALID_PROC_NUMBER)
 		{
 			if (forkNumber != MAIN_FORKNUM)
-				path = psprintf("pg_tblspc/%u/%s/%u/%u_%s",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/%u_%s",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, relNumber,
 								forkNames[forkNumber]);
 			else
-				path = psprintf("pg_tblspc/%u/%s/%u/%u",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/%u",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, relNumber);
 		}
 		else
 		{
 			if (forkNumber != MAIN_FORKNUM)
-				path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u_%s",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/t%d_%u_%s",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, procNumber, relNumber,
 								forkNames[forkNumber]);
 			else
-				path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/t%d_%u",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, procNumber, relNumber);
 		}
 	}
diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c
index c9a030853b..98fc36a033 100644
--- a/src/fe_utils/astreamer_file.c
+++ b/src/fe_utils/astreamer_file.c
@@ -19,6 +19,7 @@
 
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/relpath.h"
 #include "common/string.h"
 #include "fe_utils/astreamer.h"
 
@@ -295,23 +296,25 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member,
 static bool
 should_allow_existing_directory(const char *pathname)
 {
+#define PG_TBLSPC_DIR_CONCAT "/" PG_TBLSPC_DIR "/"
 	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, "summaries") == 0 ||
-		strcmp(filename, "pg_tblspc") == 0)
+		strcmp(filename, PG_TBLSPC_DIR) == 0)
 		return true;
 
 	if (strspn(filename, "0123456789") == strlen(filename))
 	{
-		const char *pg_tblspc = strstr(pathname, "/pg_tblspc/");
+		const char *pg_tblspc = strstr(pathname, PG_TBLSPC_DIR_CONCAT);
 
 		return pg_tblspc != NULL && pg_tblspc + 11 == filename;
 	}
 
 	return false;
+#undef PG_TBLSPC_DIR_CONCAT
 }
 
 /*
diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h
index 6f006d5a93..b0b463e9aa 100644
--- a/src/include/common/relpath.h
+++ b/src/include/common/relpath.h
@@ -33,6 +33,14 @@ typedef Oid RelFileNumber;
 #define TABLESPACE_VERSION_DIRECTORY	"PG_" PG_MAJORVERSION "_" \
 									CppAsString2(CATALOG_VERSION_NO)
 
+/*
+ * Don't change the value of those pg_tblspc related macros, as many tools
+ * probably rely on it.
+ */
+#define PG_TBLSPC_DIR "pg_tblspc"
+#define RELATIVE_PG_TBLSPC_DIR "./pg_tblspc"
+#define PG_TBLSPC_DIR_SLASH "pg_tblspc/" /* needed for strings manipulations */
+
 /* Characters to allow for an OID in a relation path */
 #define OIDCHARS		10		/* max chars printed by %u */
 
-- 
2.34.1

#18

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Yugo Nagata (#15)

Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Aug 20, 2024 at 09:30:48PM +0900, Yugo Nagata wrote:

Should be the follwing also rewritten using sizeof(PG_REPLSLOT_DIR)?

struct stat statbuf;
char path[MAXPGPATH * 2 + 12];

Yeah, done in v3 that I just shared up-thread (also removed the macros from
the comments).

Thanks!

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#19

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Bertrand Drouvot (#13)

Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Aug 20, 2024 at 12:06:52PM +0000, Bertrand Drouvot wrote:

Hi,

On Tue, Aug 20, 2024 at 05:41:48PM +0900, Michael Paquier wrote:

On Tue, Aug 20, 2024 at 11:10:46AM +0530, Ashutosh Bapat wrote:

Since these are all related changes, doing them at once might make it
faster. You may use multiple commits (one for each change)

Doing multiple commits with individual definitions for each path would
be the way to go for me. All that is mechanical, still that feels
slightly cleaner.

Right, that's what v2 has done. If there is a need for v3 then I'll add one
dedicated patch for Ashutosh's proposal in the v3 series.

Ashutosh's idea is implemented in v3 that I just shared up-thread (I used
REPLSLOT_DIR_ARGS though).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#20

ashutosh.bapat.oss@gmail.com

over 1 year ago

In reply to: Bertrand Drouvot (#19)

Re: define PG_REPLSLOT_DIR

If this isn't in commitfest already, please add it there.

On Tue, Aug 20, 2024 at 9:58 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Tue, Aug 20, 2024 at 12:06:52PM +0000, Bertrand Drouvot wrote:

Hi,

On Tue, Aug 20, 2024 at 05:41:48PM +0900, Michael Paquier wrote:

On Tue, Aug 20, 2024 at 11:10:46AM +0530, Ashutosh Bapat wrote:

Since these are all related changes, doing them at once might make it
faster. You may use multiple commits (one for each change)

Doing multiple commits with individual definitions for each path would
be the way to go for me. All that is mechanical, still that feels
slightly cleaner.

Right, that's what v2 has done. If there is a need for v3 then I'll add one
dedicated patch for Ashutosh's proposal in the v3 series.

Ashutosh's idea is implemented in v3 that I just shared up-thread (I used
REPLSLOT_DIR_ARGS though).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

--
Best Wishes,
Ashutosh Bapat

#21

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Ashutosh Bapat (#20)

Re: define PG_REPLSLOT_DIR

Hi,

On Wed, Aug 21, 2024 at 10:14:20AM +0530, Ashutosh Bapat wrote:

If this isn't in commitfest already, please add it there.

Done in [0]https://commitfest.postgresql.org/49/5193/.

[0]: https://commitfest.postgresql.org/49/5193/

Thanks!

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#22

michael@paquier.xyz

over 1 year ago

In reply to: Bertrand Drouvot (#17)

Re: define PG_REPLSLOT_DIR

On Tue, Aug 20, 2024 at 04:23:06PM +0000, Bertrand Drouvot wrote:

Please find attached v3 that:

- takes care of your comments (and also removed the use of PG_TBLSPC_DIR in
RELATIVE_PG_TBLSPC_DIR).
- removes the new macros from the comments (see Michael's and Yugo-San's
comments in [0] resp. [1]).
- adds a missing sizeof() (see [1]).
- implements Ashutosh's idea of adding a new SLOT_DIRNAME_ARGS (see [2]). It's
done in 0002 (I used REPLSLOT_DIR_ARGS though).
- fixed a macro usage in ReorderBufferCleanupSerializedTXNs() (was not at the
right location, discovered while implementing 0002).

I was looking at that, and applied 0001 for pg_replslot and merged
0003~0005 together for pg_logical. For the first one, at the end I
have updated the comments in genfile.c. For the second one, it looked
a bit strange to ignore "pg_logical/", which is the base for all the
others, so I have added a variable for it. Locating them in
reorderbuffer.h with the GUCs was OK, but perhaps there was an
argument for logical.h. The paths in origin.c refer to files, not
directories.

Not sure that 0002 is an improvement overall, so I have left this part
out.

In 0006, I am not sure that RELATIVE_PG_TBLSPC_DIR is really something
we should have. Sure, that's a special case for base backups when
sending a directory, but we have also pg_wal/ and its XLOGDIR so
that's inconsistent. Perhaps this part should be left as-is.
--
Michael

#23

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Michael Paquier (#22)

1 attachment(s)

Re: define PG_REPLSLOT_DIR

Hi,

On Fri, Aug 30, 2024 at 03:34:56PM +0900, Michael Paquier wrote:

On Tue, Aug 20, 2024 at 04:23:06PM +0000, Bertrand Drouvot wrote:

Please find attached v3 that:

- takes care of your comments (and also removed the use of PG_TBLSPC_DIR in
RELATIVE_PG_TBLSPC_DIR).
- removes the new macros from the comments (see Michael's and Yugo-San's
comments in [0] resp. [1]).
- adds a missing sizeof() (see [1]).
- implements Ashutosh's idea of adding a new SLOT_DIRNAME_ARGS (see [2]). It's
done in 0002 (I used REPLSLOT_DIR_ARGS though).
- fixed a macro usage in ReorderBufferCleanupSerializedTXNs() (was not at the
right location, discovered while implementing 0002).

I was looking at that, and applied 0001 for pg_replslot and merged
0003~0005 together for pg_logical.

Thanks!

In 0006, I am not sure that RELATIVE_PG_TBLSPC_DIR is really something
we should have. Sure, that's a special case for base backups when
sending a directory, but we have also pg_wal/ and its XLOGDIR so
that's inconsistent.

That's right but OTOH there is no (for good reason) PG_WAL_DIR or such defined.

That said, I don't have a strong opinion on this one, I think that also makes
sense to leave it as it is. Please find attached v4 doing so.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v4-0001-Define-PG_TBLSPC_DIR.patchtext/x-diff; charset=us-asciiDownload

From 4736e0913933284bbcbb81228d4a1ea4ce1fb9da Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Mon, 19 Aug 2024 07:40:10 +0000
Subject: [PATCH v4] Define PG_TBLSPC_DIR

Replace most of the places where "pg_tblspc" is used in .c files with a new
PG_TBLSPC_DIR define. The places where it is not done is for consistency with
the existing PG_STAT_TMP_DIR define.
---
 src/backend/access/transam/xlog.c           | 12 ++++-----
 src/backend/access/transam/xlogrecovery.c   | 14 +++++-----
 src/backend/backup/backup_manifest.c        |  3 ++-
 src/backend/backup/basebackup.c             |  2 +-
 src/backend/commands/dbcommands.c           |  2 +-
 src/backend/commands/tablespace.c           |  4 +--
 src/backend/storage/file/fd.c               | 29 +++++++++++----------
 src/backend/storage/file/reinit.c           | 10 +++----
 src/backend/utils/adt/dbsize.c              |  8 +++---
 src/backend/utils/adt/misc.c                |  4 +--
 src/backend/utils/cache/relcache.c          |  4 +--
 src/bin/pg_checksums/pg_checksums.c         |  6 ++---
 src/bin/pg_combinebackup/pg_combinebackup.c | 18 ++++++-------
 src/bin/pg_rewind/file_ops.c                |  2 +-
 src/bin/pg_upgrade/exec.c                   |  2 +-
 src/common/file_utils.c                     |  3 ++-
 src/common/relpath.c                        | 20 +++++++-------
 src/fe_utils/astreamer_file.c               |  7 +++--
 src/include/common/relpath.h                |  7 +++++
 19 files changed, 85 insertions(+), 72 deletions(-)
  15.5% src/backend/access/transam/
   3.3% src/backend/backup/
   4.4% src/backend/commands/
  26.8% src/backend/storage/file/
   8.8% src/backend/utils/adt/
   4.2% src/bin/pg_checksums/
  11.7% src/bin/pg_combinebackup/
  13.3% src/common/
   3.6% src/fe_utils/
   7.8% src/

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ee0fb0e28f..4e06d86196 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8944,10 +8944,10 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 		datadirpathlen = strlen(DataDir);
 
 		/* Collect information about all tablespaces */
-		tblspcdir = AllocateDir("pg_tblspc");
-		while ((de = ReadDir(tblspcdir, "pg_tblspc")) != NULL)
+		tblspcdir = AllocateDir(PG_TBLSPC_DIR);
+		while ((de = ReadDir(tblspcdir, PG_TBLSPC_DIR)) != NULL)
 		{
-			char		fullpath[MAXPGPATH + 10];
+			char		fullpath[MAXPGPATH + sizeof(PG_TBLSPC_DIR)];
 			char		linkpath[MAXPGPATH];
 			char	   *relpath = NULL;
 			char	   *s;
@@ -8970,7 +8970,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 			if (*badp != '\0' || errno == EINVAL || errno == ERANGE)
 				continue;
 
-			snprintf(fullpath, sizeof(fullpath), "pg_tblspc/%s", de->d_name);
+			snprintf(fullpath, sizeof(fullpath), "%s/%s", PG_TBLSPC_DIR, de->d_name);
 
 			de_type = get_dirent_type(fullpath, de, false, ERROR);
 
@@ -9031,8 +9031,8 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 				 * 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);
+				snprintf(linkpath, sizeof(linkpath), "%s/%s",
+						 PG_TBLSPC_DIR, de->d_name);
 				relpath = pstrdup(linkpath);
 			}
 			else
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index ad817fbca6..5d2755ef79 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -677,7 +677,7 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
 				tablespaceinfo *ti = lfirst(lc);
 				char	   *linkloc;
 
-				linkloc = psprintf("pg_tblspc/%u", ti->oid);
+				linkloc = psprintf("%s/%u", PG_TBLSPC_DIR, ti->oid);
 
 				/*
 				 * Remove the existing symlink if any and Create the symlink
@@ -2157,23 +2157,23 @@ CheckTablespaceDirectory(void)
 	DIR		   *dir;
 	struct dirent *de;
 
-	dir = AllocateDir("pg_tblspc");
-	while ((de = ReadDir(dir, "pg_tblspc")) != NULL)
+	dir = AllocateDir(PG_TBLSPC_DIR);
+	while ((de = ReadDir(dir, PG_TBLSPC_DIR)) != NULL)
 	{
-		char		path[MAXPGPATH + 10];
+		char		path[MAXPGPATH + sizeof(PG_TBLSPC_DIR)];
 
 		/* Skip entries of non-oid names */
 		if (strspn(de->d_name, "0123456789") != strlen(de->d_name))
 			continue;
 
-		snprintf(path, sizeof(path), "pg_tblspc/%s", de->d_name);
+		snprintf(path, sizeof(path), "%s/%s", PG_TBLSPC_DIR, de->d_name);
 
 		if (get_dirent_type(path, de, false, ERROR) != PGFILETYPE_LNK)
 			ereport(allow_in_place_tablespaces ? WARNING : PANIC,
 					(errcode(ERRCODE_DATA_CORRUPTED),
 					 errmsg("unexpected directory entry \"%s\" found in %s",
-							de->d_name, "pg_tblspc/"),
-					 errdetail("All directory entries in pg_tblspc/ should be symbolic links."),
+							de->d_name, PG_TBLSPC_DIR),
+					 errdetail("All directory entries in %s/ should be symbolic links.", PG_TBLSPC_DIR),
 					 errhint("Remove those directories, or set \"allow_in_place_tablespaces\" to ON transiently to let recovery complete.")));
 	}
 }
diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c
index 4357cfa31d..a2e2f86332 100644
--- a/src/backend/backup/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -16,6 +16,7 @@
 #include "access/xlog.h"
 #include "backup/backup_manifest.h"
 #include "backup/basebackup_sink.h"
+#include "common/relpath.h"
 #include "mb/pg_wchar.h"
 #include "utils/builtins.h"
 #include "utils/json.h"
@@ -117,7 +118,7 @@ AddFileToBackupManifest(backup_manifest_info *manifest, Oid spcoid,
 	 */
 	if (OidIsValid(spcoid))
 	{
-		snprintf(pathbuf, sizeof(pathbuf), "pg_tblspc/%u/%s", spcoid,
+		snprintf(pathbuf, sizeof(pathbuf), "%s/%u/%s", PG_TBLSPC_DIR, spcoid,
 				 pathname);
 		pathname = pathbuf;
 	}
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index de16afac74..14e5ba72e9 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -1488,7 +1488,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
 				if (OidIsValid(spcoid))
 				{
 					relspcoid = spcoid;
-					lookup_path = psprintf("pg_tblspc/%u/%s", spcoid,
+					lookup_path = psprintf("%s/%u/%s", PG_TBLSPC_DIR, spcoid,
 										   tarfilename);
 				}
 				else
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index d00ae40e19..8be435a79e 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -3257,7 +3257,7 @@ recovery_create_dbdir(char *path, bool only_tblspc)
 	if (stat(path, &st) == 0)
 		return;
 
-	if (only_tblspc && strstr(path, "pg_tblspc/") == NULL)
+	if (only_tblspc && strstr(path, PG_TBLSPC_DIR_SLASH) == NULL)
 		elog(PANIC, "requested to created invalid directory: %s", path);
 
 	if (reachedConsistency && !allow_in_place_tablespaces)
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 113b480731..00c1ed19fd 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -576,7 +576,7 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
 	struct stat st;
 	bool		in_place;
 
-	linkloc = psprintf("pg_tblspc/%u", tablespaceoid);
+	linkloc = psprintf("%s/%u", PG_TBLSPC_DIR, tablespaceoid);
 
 	/*
 	 * If we're asked to make an 'in place' tablespace, create the directory
@@ -692,7 +692,7 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo)
 	char	   *subfile;
 	struct stat st;
 
-	linkloc_with_version_dir = psprintf("pg_tblspc/%u/%s", tablespaceoid,
+	linkloc_with_version_dir = psprintf("%s/%u/%s", PG_TBLSPC_DIR, tablespaceoid,
 										TABLESPACE_VERSION_DIRECTORY);
 
 	/*
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 368cc9455c..d5204b1eb9 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1790,8 +1790,8 @@ TempTablespacePath(char *path, Oid tablespace)
 	else
 	{
 		/* All other tablespaces are accessed via symlinks */
-		snprintf(path, MAXPGPATH, "pg_tblspc/%u/%s/%s",
-				 tablespace, TABLESPACE_VERSION_DIRECTORY,
+		snprintf(path, MAXPGPATH, "%s/%u/%s/%s",
+				 PG_TBLSPC_DIR, tablespace, TABLESPACE_VERSION_DIRECTORY,
 				 PG_TEMP_FILES_DIR);
 	}
 }
@@ -3296,7 +3296,7 @@ CleanupTempFiles(bool isCommit, bool isProcExit)
 void
 RemovePgTempFiles(void)
 {
-	char		temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY) + sizeof(PG_TEMP_FILES_DIR)];
+	char		temp_path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY) + sizeof(PG_TEMP_FILES_DIR)];
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 
@@ -3310,20 +3310,21 @@ RemovePgTempFiles(void)
 	/*
 	 * Cycle through temp directories for all non-default tablespaces.
 	 */
-	spc_dir = AllocateDir("pg_tblspc");
+	spc_dir = AllocateDir(PG_TBLSPC_DIR);
 
-	while ((spc_de = ReadDirExtended(spc_dir, "pg_tblspc", LOG)) != NULL)
+	while ((spc_de = ReadDirExtended(spc_dir, PG_TBLSPC_DIR, LOG)) != NULL)
 	{
 		if (strcmp(spc_de->d_name, ".") == 0 ||
 			strcmp(spc_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY, PG_TEMP_FILES_DIR);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY,
+				 PG_TEMP_FILES_DIR);
 		RemovePgTempFilesInDir(temp_path, true, false);
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
 		RemovePgTempRelationFiles(temp_path);
 	}
 
@@ -3610,15 +3611,15 @@ SyncDataDirectory(void)
 		/* Sync the top level pgdata directory. */
 		do_syncfs(".");
 		/* If any tablespaces are configured, sync each of those. */
-		dir = AllocateDir("pg_tblspc");
-		while ((de = ReadDirExtended(dir, "pg_tblspc", LOG)))
+		dir = AllocateDir(PG_TBLSPC_DIR);
+		while ((de = ReadDirExtended(dir, PG_TBLSPC_DIR, LOG)))
 		{
 			char		path[MAXPGPATH];
 
 			if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
 				continue;
 
-			snprintf(path, MAXPGPATH, "pg_tblspc/%s", de->d_name);
+			snprintf(path, MAXPGPATH, "%s/%s", PG_TBLSPC_DIR, de->d_name);
 			do_syncfs(path);
 		}
 		FreeDir(dir);
@@ -3641,7 +3642,7 @@ SyncDataDirectory(void)
 	walkdir(".", pre_sync_fname, false, DEBUG1);
 	if (xlog_is_symlink)
 		walkdir("pg_wal", pre_sync_fname, false, DEBUG1);
-	walkdir("pg_tblspc", pre_sync_fname, true, DEBUG1);
+	walkdir(PG_TBLSPC_DIR, pre_sync_fname, true, DEBUG1);
 #endif
 
 	/* Prepare to report progress syncing the data directory via fsync. */
@@ -3659,7 +3660,7 @@ SyncDataDirectory(void)
 	walkdir(".", datadir_fsync_fname, false, LOG);
 	if (xlog_is_symlink)
 		walkdir("pg_wal", datadir_fsync_fname, false, LOG);
-	walkdir("pg_tblspc", datadir_fsync_fname, true, LOG);
+	walkdir(PG_TBLSPC_DIR, datadir_fsync_fname, true, LOG);
 }
 
 /*
diff --git a/src/backend/storage/file/reinit.c b/src/backend/storage/file/reinit.c
index f1cd1a38d9..01e267abf9 100644
--- a/src/backend/storage/file/reinit.c
+++ b/src/backend/storage/file/reinit.c
@@ -46,7 +46,7 @@ typedef struct
 void
 ResetUnloggedRelations(int op)
 {
-	char		temp_path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)];
+	char		temp_path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY)];
 	DIR		   *spc_dir;
 	struct dirent *spc_de;
 	MemoryContext tmpctx,
@@ -77,16 +77,16 @@ ResetUnloggedRelations(int op)
 	/*
 	 * Cycle through directories for all non-default tablespaces.
 	 */
-	spc_dir = AllocateDir("pg_tblspc");
+	spc_dir = AllocateDir(PG_TBLSPC_DIR);
 
-	while ((spc_de = ReadDir(spc_dir, "pg_tblspc")) != NULL)
+	while ((spc_de = ReadDir(spc_dir, PG_TBLSPC_DIR)) != NULL)
 	{
 		if (strcmp(spc_de->d_name, ".") == 0 ||
 			strcmp(spc_de->d_name, "..") == 0)
 			continue;
 
-		snprintf(temp_path, sizeof(temp_path), "pg_tblspc/%s/%s",
-				 spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
+		snprintf(temp_path, sizeof(temp_path), "%s/%s/%s",
+				 PG_TBLSPC_DIR, spc_de->d_name, TABLESPACE_VERSION_DIRECTORY);
 		ResetUnloggedRelationsInTablespaceDir(temp_path, op);
 	}
 
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index b2d9cc2792..e63e99c141 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -143,7 +143,7 @@ calculate_database_size(Oid dbOid)
 	totalsize = db_dir_size(pathname);
 
 	/* Scan the non-default tablespaces */
-	snprintf(dirpath, MAXPGPATH, "pg_tblspc");
+	snprintf(dirpath, MAXPGPATH, PG_TBLSPC_DIR);
 	dirdesc = AllocateDir(dirpath);
 
 	while ((direntry = ReadDir(dirdesc, dirpath)) != NULL)
@@ -154,8 +154,8 @@ calculate_database_size(Oid dbOid)
 			strcmp(direntry->d_name, "..") == 0)
 			continue;
 
-		snprintf(pathname, sizeof(pathname), "pg_tblspc/%s/%s/%u",
-				 direntry->d_name, TABLESPACE_VERSION_DIRECTORY, dbOid);
+		snprintf(pathname, sizeof(pathname), "%s/%s/%s/%u",
+				 PG_TBLSPC_DIR, direntry->d_name, TABLESPACE_VERSION_DIRECTORY, dbOid);
 		totalsize += db_dir_size(pathname);
 	}
 
@@ -227,7 +227,7 @@ calculate_tablespace_size(Oid tblspcOid)
 	else if (tblspcOid == GLOBALTABLESPACE_OID)
 		snprintf(tblspcPath, MAXPGPATH, "global");
 	else
-		snprintf(tblspcPath, MAXPGPATH, "pg_tblspc/%u/%s", tblspcOid,
+		snprintf(tblspcPath, MAXPGPATH, "%s/%u/%s", PG_TBLSPC_DIR, tblspcOid,
 				 TABLESPACE_VERSION_DIRECTORY);
 
 	dirdesc = AllocateDir(tblspcPath);
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 0e6c45807a..1aa8bbb306 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -242,7 +242,7 @@ pg_tablespace_databases(PG_FUNCTION_ARGS)
 	if (tablespaceOid == DEFAULTTABLESPACE_OID)
 		location = "base";
 	else
-		location = psprintf("pg_tblspc/%u/%s", tablespaceOid,
+		location = psprintf("%s/%u/%s", PG_TBLSPC_DIR, tablespaceOid,
 							TABLESPACE_VERSION_DIRECTORY);
 
 	dirdesc = AllocateDir(location);
@@ -325,7 +325,7 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
 	 * Find the location of the tablespace by reading the symbolic link that
 	 * is in pg_tblspc/<oid>.
 	 */
-	snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%u", tablespaceOid);
+	snprintf(sourcepath, sizeof(sourcepath), "%s/%u", PG_TBLSPC_DIR, tablespaceOid);
 
 	/*
 	 * Before reading the link, check if the source path is a link or a
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 66ed24e401..63efc55f09 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -6800,10 +6800,10 @@ RelationCacheInitFilePostInvalidate(void)
 void
 RelationCacheInitFileRemove(void)
 {
-	const char *tblspcdir = "pg_tblspc";
+	const char *tblspcdir = PG_TBLSPC_DIR;
 	DIR		   *dir;
 	struct dirent *de;
-	char		path[MAXPGPATH + 10 + sizeof(TABLESPACE_VERSION_DIRECTORY)];
+	char		path[MAXPGPATH + sizeof(PG_TBLSPC_DIR) + sizeof(TABLESPACE_VERSION_DIRECTORY)];
 
 	snprintf(path, sizeof(path), "global/%s",
 			 RELCACHE_INIT_FILENAME);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index b5bb0e7887..68a68eb0fa 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -388,7 +388,7 @@ scan_directory(const char *basedir, const char *subdir, bool sizeonly)
 			 * is valid, resolving the linked locations and dive into them
 			 * directly.
 			 */
-			if (strncmp("pg_tblspc", subdir, strlen("pg_tblspc")) == 0)
+			if (strncmp(PG_TBLSPC_DIR, subdir, strlen(PG_TBLSPC_DIR)) == 0)
 			{
 				char		tblspc_path[MAXPGPATH];
 				struct stat tblspc_st;
@@ -593,12 +593,12 @@ main(int argc, char *argv[])
 		{
 			total_size = scan_directory(DataDir, "global", true);
 			total_size += scan_directory(DataDir, "base", true);
-			total_size += scan_directory(DataDir, "pg_tblspc", true);
+			total_size += scan_directory(DataDir, PG_TBLSPC_DIR, true);
 		}
 
 		(void) scan_directory(DataDir, "global", false);
 		(void) scan_directory(DataDir, "base", false);
-		(void) scan_directory(DataDir, "pg_tblspc", false);
+		(void) scan_directory(DataDir, PG_TBLSPC_DIR, false);
 
 		if (showprogress)
 			progress_report(true);
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 5e2f4f4b3d..05a2ff905b 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -373,7 +373,7 @@ main(int argc, char *argv[])
 		{
 			char		linkpath[MAXPGPATH];
 
-			snprintf(linkpath, MAXPGPATH, "%s/pg_tblspc/%u", opt.output,
+			snprintf(linkpath, MAXPGPATH, "%s/%s/%u", opt.output, PG_TBLSPC_DIR,
 					 ts->oid);
 
 			if (opt.dry_run)
@@ -867,12 +867,12 @@ process_directory_recursively(Oid tsoid,
 		is_incremental_dir = true;
 	else if (relative_path != NULL)
 	{
-		is_pg_tblspc = strcmp(relative_path, "pg_tblspc") == 0;
+		is_pg_tblspc = strcmp(relative_path, PG_TBLSPC_DIR) == 0;
 		is_pg_wal = (strcmp(relative_path, "pg_wal") == 0 ||
 					 strncmp(relative_path, "pg_wal/", 7) == 0);
 		is_incremental_dir = strncmp(relative_path, "base/", 5) == 0 ||
 			strcmp(relative_path, "global") == 0 ||
-			strncmp(relative_path, "pg_tblspc/", 10) == 0;
+			strncmp(relative_path, PG_TBLSPC_DIR_SLASH, 10) == 0;
 	}
 
 	/*
@@ -895,7 +895,7 @@ process_directory_recursively(Oid tsoid,
 		strlcpy(ifulldir, input_directory, MAXPGPATH);
 		strlcpy(ofulldir, output_directory, MAXPGPATH);
 		if (OidIsValid(tsoid))
-			snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/", tsoid);
+			snprintf(manifest_prefix, MAXPGPATH, "%s/%u/", PG_TBLSPC_DIR, tsoid);
 		else
 			manifest_prefix[0] = '\0';
 	}
@@ -906,8 +906,8 @@ process_directory_recursively(Oid tsoid,
 		snprintf(ofulldir, MAXPGPATH, "%s/%s", output_directory,
 				 relative_path);
 		if (OidIsValid(tsoid))
-			snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/%s/",
-					 tsoid, relative_path);
+			snprintf(manifest_prefix, MAXPGPATH, "%s/%u/%s/",
+					 PG_TBLSPC_DIR, tsoid, relative_path);
 		else
 			snprintf(manifest_prefix, MAXPGPATH, "%s/", relative_path);
 	}
@@ -1249,7 +1249,7 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 	struct dirent *de;
 	cb_tablespace *tslist = NULL;
 
-	snprintf(pg_tblspc, MAXPGPATH, "%s/pg_tblspc", pathname);
+	snprintf(pg_tblspc, MAXPGPATH, "%s/%s", pathname, PG_TBLSPC_DIR);
 	pg_log_debug("scanning \"%s\"", pg_tblspc);
 
 	if ((dir = opendir(pg_tblspc)) == NULL)
@@ -1344,8 +1344,8 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
 			 * we just record the paths within the data directories.
 			 */
 			snprintf(ts->old_dir, MAXPGPATH, "%s/%s", pg_tblspc, de->d_name);
-			snprintf(ts->new_dir, MAXPGPATH, "%s/pg_tblspc/%s", opt->output,
-					 de->d_name);
+			snprintf(ts->new_dir, MAXPGPATH, "%s/%s/%s", opt->output,
+					 PG_TBLSPC_DIR, de->d_name);
 			ts->in_place = true;
 		}
 
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index d93580bb41..67a86bb4c5 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -452,7 +452,7 @@ recurse_dir(const char *datadir, const char *parentpath,
 			 * to process all the tablespaces.  We also follow a symlink if
 			 * it's for pg_wal.  Symlinks elsewhere are ignored.
 			 */
-			if ((parentpath && strcmp(parentpath, "pg_tblspc") == 0) ||
+			if ((parentpath && strcmp(parentpath, PG_TBLSPC_DIR) == 0) ||
 				strcmp(path, "pg_wal") == 0)
 				recurse_dir(datadir, path, callback);
 		}
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 058530ab3e..78db321ace 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -350,7 +350,7 @@ check_data_dir(ClusterInfo *cluster)
 	check_single_dir(pg_data, "global");
 	check_single_dir(pg_data, "pg_multixact");
 	check_single_dir(pg_data, "pg_subtrans");
-	check_single_dir(pg_data, "pg_tblspc");
+	check_single_dir(pg_data, PG_TBLSPC_DIR);
 	check_single_dir(pg_data, "pg_twophase");
 
 	/* pg_xlog has been renamed to pg_wal in v10 */
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 6bac537a1e..761873d519 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 
 #include "common/file_utils.h"
+#include "common/relpath.h"
 #ifdef FRONTEND
 #include "common/logging.h"
 #endif
@@ -105,7 +106,7 @@ sync_pgdata(const char *pg_data,
 	/* handle renaming of pg_xlog to pg_wal in post-10 clusters */
 	snprintf(pg_wal, MAXPGPATH, "%s/%s", pg_data,
 			 serverVersion < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal");
-	snprintf(pg_tblspc, MAXPGPATH, "%s/pg_tblspc", pg_data);
+	snprintf(pg_tblspc, MAXPGPATH, "%s/%s", pg_data, PG_TBLSPC_DIR);
 
 	/*
 	 * If pg_wal is a symlink, we'll need to recurse into it separately,
diff --git a/src/common/relpath.c b/src/common/relpath.c
index f54c36ef7a..426a0114f8 100644
--- a/src/common/relpath.c
+++ b/src/common/relpath.c
@@ -123,8 +123,8 @@ GetDatabasePath(Oid dbOid, Oid spcOid)
 	else
 	{
 		/* All other tablespaces are accessed via symlinks */
-		return psprintf("pg_tblspc/%u/%s/%u",
-						spcOid, TABLESPACE_VERSION_DIRECTORY, dbOid);
+		return psprintf("%s/%u/%s/%u",
+						PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY, dbOid);
 	}
 }
 
@@ -184,25 +184,25 @@ GetRelationPath(Oid dbOid, Oid spcOid, RelFileNumber relNumber,
 		if (procNumber == INVALID_PROC_NUMBER)
 		{
 			if (forkNumber != MAIN_FORKNUM)
-				path = psprintf("pg_tblspc/%u/%s/%u/%u_%s",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/%u_%s",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, relNumber,
 								forkNames[forkNumber]);
 			else
-				path = psprintf("pg_tblspc/%u/%s/%u/%u",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/%u",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, relNumber);
 		}
 		else
 		{
 			if (forkNumber != MAIN_FORKNUM)
-				path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u_%s",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/t%d_%u_%s",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, procNumber, relNumber,
 								forkNames[forkNumber]);
 			else
-				path = psprintf("pg_tblspc/%u/%s/%u/t%d_%u",
-								spcOid, TABLESPACE_VERSION_DIRECTORY,
+				path = psprintf("%s/%u/%s/%u/t%d_%u",
+								PG_TBLSPC_DIR, spcOid, TABLESPACE_VERSION_DIRECTORY,
 								dbOid, procNumber, relNumber);
 		}
 	}
diff --git a/src/fe_utils/astreamer_file.c b/src/fe_utils/astreamer_file.c
index c9a030853b..98fc36a033 100644
--- a/src/fe_utils/astreamer_file.c
+++ b/src/fe_utils/astreamer_file.c
@@ -19,6 +19,7 @@
 
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/relpath.h"
 #include "common/string.h"
 #include "fe_utils/astreamer.h"
 
@@ -295,23 +296,25 @@ astreamer_extractor_content(astreamer *streamer, astreamer_member *member,
 static bool
 should_allow_existing_directory(const char *pathname)
 {
+#define PG_TBLSPC_DIR_CONCAT "/" PG_TBLSPC_DIR "/"
 	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, "summaries") == 0 ||
-		strcmp(filename, "pg_tblspc") == 0)
+		strcmp(filename, PG_TBLSPC_DIR) == 0)
 		return true;
 
 	if (strspn(filename, "0123456789") == strlen(filename))
 	{
-		const char *pg_tblspc = strstr(pathname, "/pg_tblspc/");
+		const char *pg_tblspc = strstr(pathname, PG_TBLSPC_DIR_CONCAT);
 
 		return pg_tblspc != NULL && pg_tblspc + 11 == filename;
 	}
 
 	return false;
+#undef PG_TBLSPC_DIR_CONCAT
 }
 
 /*
diff --git a/src/include/common/relpath.h b/src/include/common/relpath.h
index 6f006d5a93..6d83f90510 100644
--- a/src/include/common/relpath.h
+++ b/src/include/common/relpath.h
@@ -33,6 +33,13 @@ typedef Oid RelFileNumber;
 #define TABLESPACE_VERSION_DIRECTORY	"PG_" PG_MAJORVERSION "_" \
 									CppAsString2(CATALOG_VERSION_NO)
 
+/*
+ * Don't change the value of those pg_tblspc related macros, as many tools
+ * probably rely on it.
+ */
+#define PG_TBLSPC_DIR "pg_tblspc"
+#define PG_TBLSPC_DIR_SLASH "pg_tblspc/" /* needed for strings manipulations */
+
 /* Characters to allow for an OID in a relation path */
 #define OIDCHARS		10		/* max chars printed by %u */
 
-- 
2.34.1

#24

michael@paquier.xyz

over 1 year ago

In reply to: Bertrand Drouvot (#23)

Re: define PG_REPLSLOT_DIR

On Fri, Aug 30, 2024 at 12:21:29PM +0000, Bertrand Drouvot wrote:

That said, I don't have a strong opinion on this one, I think that also makes
sense to leave it as it is. Please find attached v4 doing so.

The changes in astreamer_file.c are actually wrong regarding the fact
that should_allow_existing_directory() needs to be able to work with
the branch where this code is located as well as back-branches,
because pg_basebackup from version N supports ~(N-1) versions down to
a certain version, so changing it is not right. This is why pg_xlog
and pg_wal are both listed there.

Perhaps we should to more for the two entries in basebackup.c with the
relative paths, but I'm not sure that's worth bothering, either. At
the end, I got no objections about the remaining pieces, so applied.

How do people feel about the suggestions to update the comments at the
end? With the comment in relpath.h suggesting to not change that, the
current state of HEAD is fine by me.
--
Michael

#25

bertranddrouvot.pg@gmail.com

over 1 year ago

In reply to: Michael Paquier (#24)

Re: define PG_REPLSLOT_DIR

Hi,

On Tue, Sep 03, 2024 at 09:15:50AM +0900, Michael Paquier wrote:

On Fri, Aug 30, 2024 at 12:21:29PM +0000, Bertrand Drouvot wrote:

That said, I don't have a strong opinion on this one, I think that also makes
sense to leave it as it is. Please find attached v4 doing so.

The changes in astreamer_file.c are actually wrong regarding the fact
that should_allow_existing_directory() needs to be able to work with
the branch where this code is located as well as back-branches,
because pg_basebackup from version N supports ~(N-1) versions down to
a certain version, so changing it is not right. This is why pg_xlog
and pg_wal are both listed there.

I understand why pg_xlog and pg_wal both need to be listed here, but I don't
get why the proposed changes were "wrong". Or, are you saying that if for any
reason PG_TBLSPC_DIR needs to be changed that would not work anymore? If
that's the case, then I guess we'd have to add a new define and test like:

strcmp(filename, PG_TBLSPC_DIR) == 0) || strcmp(filename, NEW_PG_TBLSPC_DIR) == 0)

, no?

The question is more out of curiosity, not saying the changes should be applied
in astreamer_file.c though.

Perhaps we should to more for the two entries in basebackup.c with the
relative paths, but I'm not sure that's worth bothering, either.

I don't have a strong opinon on those ones.

At
the end, I got no objections about the remaining pieces, so applied.

Thanks!

How do people feel about the suggestions to update the comments at the
end? With the comment in relpath.h suggesting to not change that, the
current state of HEAD is fine by me.

Yeah, I think that's fine and specially because there is still some places (
outside of the comments) that don't rely on the define(s).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

#26