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>
signature.asc
Description: PGP signature