On Wed, Mar 16, 2022 at 10:34:15AM +0900, Kyotaro Horiguchi wrote:
> +1. Desn't the doc need to mention that?

Yes, I agree that it makes sense to add a note, even if
allow_in_place_tablespaces is a developer option.  I have added the
following paragraph in the docs:
+        A full path of the symbolic link in <filename>pg_tblspc/</filename>
+        is returned. A relative path to the data directory is returned
+        for tablespaces created with
+        <xref linkend="guc-allow-in-place-tablespaces"/> enabled.

Another thing that was annoying in the first version of the patch is
the useless call to lstat() on Windows, not needed because it is
possible to rely just on pgwin32_is_junction() to check if readlink()
should be called or not.

This leads me to the revised version attached.  What do you think?
--
Michael
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 4568749d23..89690be2ed 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -15,6 +15,7 @@
 #include "postgres.h"
 
 #include <sys/file.h>
+#include <sys/stat.h>
 #include <dirent.h>
 #include <fcntl.h>
 #include <math.h>
@@ -282,6 +283,9 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
 	char		sourcepath[MAXPGPATH];
 	char		targetpath[MAXPGPATH];
 	int			rllen;
+#ifndef WIN32
+	struct stat st;
+#endif
 
 	/*
 	 * It's useful to apply this function to pg_class.reltablespace, wherein
@@ -306,6 +310,31 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
 	 */
 	snprintf(sourcepath, sizeof(sourcepath), "pg_tblspc/%u", tablespaceOid);
 
+	/*
+	 * Before reading the link, check if the source path is a link or a
+	 * junction point.  Note that a directory is possible for a tablespace
+	 * created with allow_in_place_tablespaces enabled.  If a directory is
+	 * found, a relative path to the data directory is returned.
+	 */
+#ifdef WIN32
+	if (!pgwin32_is_junction(sourcepath))
+		PG_RETURN_TEXT_P(cstring_to_text(sourcepath));
+#else
+	if (lstat(sourcepath, &st) < 0)
+	{
+		ereport(ERROR,
+				(errcode_for_file_access(),
+				 errmsg("could not stat file \"%s\": %m",
+						sourcepath)));
+	}
+
+	if (!S_ISLNK(st.st_mode))
+		PG_RETURN_TEXT_P(cstring_to_text(sourcepath));
+#endif
+
+	/*
+	 * In presence of a link or a junction point, return the path pointing to.
+	 */
 	rllen = readlink(sourcepath, targetpath, sizeof(targetpath));
 	if (rllen < 0)
 		ereport(ERROR,
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index 2dfbcfdebe..473fe8c28e 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -24,6 +24,15 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
 DROP TABLESPACE regress_tblspacewith;
 -- create a tablespace we can use
 CREATE TABLESPACE regress_tblspace LOCATION '';
+-- This returns a relative path as of an effect of allow_in_place_tablespaces,
+-- masking the tablespace OID used in the path name.
+SELECT regexp_replace(pg_tablespace_location(oid), '(pg_tblspc)/(\d+)', '\1/NNN')
+  FROM pg_tablespace  WHERE spcname = 'regress_tblspace';
+ regexp_replace 
+----------------
+ pg_tblspc/NNN
+(1 row)
+
 -- try setting and resetting some properties for the new tablespace
 ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1);
 ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true);  -- fail
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index 896f05cea3..0949a28488 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -22,6 +22,10 @@ DROP TABLESPACE regress_tblspacewith;
 
 -- create a tablespace we can use
 CREATE TABLESPACE regress_tblspace LOCATION '';
+-- This returns a relative path as of an effect of allow_in_place_tablespaces,
+-- masking the tablespace OID used in the path name.
+SELECT regexp_replace(pg_tablespace_location(oid), '(pg_tblspc)/(\d+)', '\1/NNN')
+  FROM pg_tablespace  WHERE spcname = 'regress_tblspace';
 
 -- try setting and resetting some properties for the new tablespace
 ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1);
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8a802fb225..df54c5815d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23924,7 +23924,14 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id'));
        </para>
        <para>
         Returns the file system path that this tablespace is located in.
-       </para></entry>
+       </para>
+       <para>
+        A full path of the symbolic link in <filename>pg_tblspc/</filename>
+        is returned. A relative path to the data directory is returned
+        for tablespaces created with
+        <xref linkend="guc-allow-in-place-tablespaces"/> enabled.
+       </para>
+       </entry>
       </row>
 
       <row>

Attachment: signature.asc
Description: PGP signature

Reply via email to