Re: Parallell hashjoin sometimes ignores temp_tablespaces

Magnus Hagander <magnus@hagander.net> writes:
> On Fri, Jul 3, 2020 at 6:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The lack of documentation seems to be my fault, so I'm willing to pick
>> this up unless somebody else wants it.

> If the comments I included in that patch are enough, I can just commit
> those along with it. Otherwise, please do :)

Being once burned, I had something more like the attached in mind.

BTW, looking at this, I'm kind of wondering about the other code path
in SharedFileSetInit:

    if (fileset->ntablespaces == 0)
    {
        fileset->tablespaces[0] = DEFAULTTABLESPACE_OID;
        fileset->ntablespaces = 1;
    }

Shouldn't that be inserting MyDatabaseTableSpace?  I see no other places
anywhere that are forcing temp stuff into pg_default like this.

            regards, tom lane

diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index f887ee9857..2c3b9050b2 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -1183,6 +1183,7 @@ GetDefaultTablespace(char relpersistence, bool partitioned)

 typedef struct
 {
+    /* Array of OIDs to be passed to SetTempTablespaces() */
     int            numSpcs;
     Oid            tblSpcs[FLEXIBLE_ARRAY_MEMBER];
 } temp_tablespaces_extra;
@@ -1232,6 +1233,7 @@ check_temp_tablespaces(char **newval, void **extra, GucSource source)
             /* Allow an empty string (signifying database default) */
             if (curname[0] == '\0')
             {
+                /* InvalidOid signifies database's default tablespace */
                 tblSpcs[numSpcs++] = InvalidOid;
                 continue;
             }
@@ -1258,6 +1260,7 @@ check_temp_tablespaces(char **newval, void **extra, GucSource source)
              */
             if (curoid == MyDatabaseTableSpace)
             {
+                /* InvalidOid signifies database's default tablespace */
                 tblSpcs[numSpcs++] = InvalidOid;
                 continue;
             }
@@ -1368,6 +1371,7 @@ PrepareTempTablespaces(void)
         /* Allow an empty string (signifying database default) */
         if (curname[0] == '\0')
         {
+            /* InvalidOid signifies database's default tablespace */
             tblSpcs[numSpcs++] = InvalidOid;
             continue;
         }
@@ -1386,7 +1390,8 @@ PrepareTempTablespaces(void)
          */
         if (curoid == MyDatabaseTableSpace)
         {
-            tblSpcs[numSpcs++] = curoid;
+            /* InvalidOid signifies database's default tablespace */
+            tblSpcs[numSpcs++] = InvalidOid;
             continue;
         }

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 7dc6dd2f15..5f6420efb2 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -264,8 +264,10 @@ static int    numExternalFDs = 0;
 static long tempFileCounter = 0;

 /*
- * Array of OIDs of temp tablespaces.  When numTempTableSpaces is -1,
- * this has not been set in the current transaction.
+ * Array of OIDs of temp tablespaces.  (Some entries may be InvalidOid,
+ * indicating that the current database's default tablespace should be used.)
+ * When numTempTableSpaces is -1, this has not been set in the current
+ * transaction.
  */
 static Oid *tempTableSpaces = NULL;
 static int    numTempTableSpaces = -1;
@@ -2779,6 +2781,9 @@ closeAllVfds(void)
  * unless this function is called again before then.  It is caller's
  * responsibility that the passed-in array has adequate lifespan (typically
  * it'd be allocated in TopTransactionContext).
+ *
+ * Some entries of the array may be InvalidOid, indicating that the current
+ * database's default tablespace should be used.
  */
 void
 SetTempTablespaces(Oid *tableSpaces, int numSpaces)
@@ -2818,7 +2823,10 @@ TempTablespacesAreSet(void)
  * GetTempTablespaces
  *
  * Populate an array with the OIDs of the tablespaces that should be used for
- * temporary files.  Return the number that were copied into the output array.
+ * temporary files.  (Some entries may be InvalidOid, indicating that the
+ * current database's default tablespace should be used.)  At most numSpaces
+ * entries will be filled.
+ * Returns the number of OIDs that were copied into the output array.
  */
 int
 GetTempTablespaces(Oid *tableSpaces, int numSpaces)
diff --git a/src/backend/storage/file/sharedfileset.c b/src/backend/storage/file/sharedfileset.c
index f7206c9175..0f65210476 100644
--- a/src/backend/storage/file/sharedfileset.c
+++ b/src/backend/storage/file/sharedfileset.c
@@ -66,6 +66,21 @@ SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg)
         fileset->tablespaces[0] = DEFAULTTABLESPACE_OID;
         fileset->ntablespaces = 1;
     }
+    else
+    {
+        int            i;
+
+        /*
+         * An entry of InvalidOid means use the default tablespace for the
+         * current database.  Replace that now, to be sure that all users of
+         * the SharedFileSet agree on what to do.
+         */
+        for (i = 0; i < fileset->ntablespaces; i++)
+        {
+            if (fileset->tablespaces[i] == InvalidOid)
+                fileset->tablespaces[i] = MyDatabaseTableSpace;
+        }
+    }

     /* Register our cleanup callback. */
     on_dsm_detach(seg, SharedFileSetOnDetach, PointerGetDatum(fileset));