On Wed, Mar 24, 2021 at 07:56:59PM -0700, Noah Misch wrote:
> That would entail forbidding various shell metacharacters in @testtablespace@.
> One could avoid imposing such a restriction by adding a mkdir() function to
> regress.c and writing it like:
>
> CREATE FUNCTION mkdir(text)
> RETURNS void AS '@libdir@/regress@DLSUFFIX@', 'regress_mkdir'
> LANGUAGE C STRICT\;
> REVOKE ALL ON FUNCTION mkdir FROM public;
> SELECT mkdir('@testtablespace@');
> CREATE TABLESPACE regress_tblspace LOCATION '@testtablespace@';Sounds simple. >> I doubt that all the Windows environments would be able to get their >> hands on that. > >> And I am not sure either that this would work when it >> comes to the CI case, again on Windows. > > How might a CI provider break that? I am wondering about potential issues when it comes to create the base tablespace path from the Postgres backend, while HEAD does it while relying on a restricted token obtained when starting pg_regress. I have compiled a simple patch that uses a SQL function for the base tablespace directory creation, that I have tested on Linux and MSVC. To get some coverage with the CF bot, I am adding a CF entry with this thread. I am still not sure if people would prefer this approach over what's on HEAD. So if there are any opinions, please feel free. -- Michael
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index c133e73499..0c1a9929c2 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -1,3 +1,10 @@
+-- Create the tablespace path.
+CREATE FUNCTION mkdir(text)
+ RETURNS void AS '@libdir@/regress@DLSUFFIX@', 'regress_mkdir'
+ LANGUAGE C STRICT;
+SELECT mkdir('@testtablespace@');
+DROP FUNCTION mkdir(text);
+
-- create a tablespace using WITH clause
CREATE TABLESPACE regress_tblspacewith LOCATION '@testtablespace@' WITH (some_nonexistent_parameter = true); -- fail
CREATE TABLESPACE regress_tblspacewith LOCATION '@testtablespace@' WITH (random_page_cost = 3.0); -- ok
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 1bbe7e0323..ea07211d1a 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -1,3 +1,14 @@
+-- Create the tablespace path.
+CREATE FUNCTION mkdir(text)
+ RETURNS void AS '@libdir@/regress@DLSUFFIX@', 'regress_mkdir'
+ LANGUAGE C STRICT;
+SELECT mkdir('@testtablespace@');
+ mkdir
+-------
+
+(1 row)
+
+DROP FUNCTION mkdir(text);
-- create a tablespace using WITH clause
CREATE TABLESPACE regress_tblspacewith LOCATION '@testtablespace@' WITH (some_nonexistent_parameter = true); -- fail
ERROR: unrecognized parameter "some_nonexistent_parameter"
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index b7d80bd9bb..dbb2e8bea7 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -506,23 +506,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
- /*
- * Clean out the test tablespace dir, or create it if it doesn't exist. On
- * Windows, doing this cleanup here makes possible to run the regression
- * tests as a Windows administrative user account with the restricted
- * token obtained when starting pg_regress.
- */
- if (directory_exists(testtablespace))
- {
- if (!rmtree(testtablespace, true))
- {
- fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
- progname, testtablespace);
- exit(2);
- }
- }
- make_directory(testtablespace);
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c
index 1990cbb6a1..a884b5ed71 100644
--- a/src/test/regress/regress.c
+++ b/src/test/regress/regress.c
@@ -18,6 +18,7 @@
#include <math.h>
#include <signal.h>
+#include <sys/stat.h>
#include "access/detoast.h"
#include "access/htup_details.h"
@@ -80,6 +81,43 @@ static void regress_lseg_construct(LSEG *lseg, Point *pt1, Point *pt2);
PG_MODULE_MAGIC;
+static bool
+directory_exists(const char *dir)
+{
+ struct stat st;
+
+ if (stat(dir, &st) != 0)
+ return false;
+ if (S_ISDIR(st.st_mode))
+ return true;
+ return false;
+}
+
+/*
+ * Create a new directory for the tests, to be used for tablespaces.
+ * If this directory exists, clean up its contents and create it again
+ * from scratch.
+ */
+PG_FUNCTION_INFO_V1(regress_mkdir);
+
+Datum
+regress_mkdir(PG_FUNCTION_ARGS)
+{
+ char *dir = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+ if (!superuser())
+ elog(ERROR, "must be superuser to create paths");
+
+ if (directory_exists(dir))
+ {
+ if (!rmtree(dir, true))
+ elog(ERROR, "could not remove path \"%s\"\n", dir);
+ }
+ if (mkdir(dir, S_IRWXU | S_IRWXG | S_IRWXO) < 0)
+ elog(ERROR, "could not create directory \"%s\": %m", dir);
+
+ PG_RETURN_VOID();
+}
/* return the point where two paths intersect, or NULL if no intersection. */
PG_FUNCTION_INFO_V1(interpt_pp);
signature.asc
Description: PGP signature
