Re: Calling PrepareTempTablespaces in BufFileCreateTemp

I wrote:
> Here's a draft patch for that.
>
> It's slightly ugly that this adds a dependency on commands/tablespace
> to fd.c, which is a pretty low-level module.  I think wanting to avoid
> that layering violation might've been the reason for doing things the
> way they are.  However, this gets rid of tablespace dependencies in
> some other files that are only marginally higher-level, like
> tuplesort.c, so I'm not sure how strong that objection is.
>
> There are three functions in fd.c that have a dependency on the
> temp tablespace info having been set up:
>     OpenTemporaryFile
>     GetTempTablespaces
>     GetNextTempTableSpace
> This patch makes the first of those automatically set up the info
> if it's not done yet.  The second one has always had an assertion
> that the caller did it already, and now the third one does too.

After a bit more thought it seemed like another answer would be to
make all three of those functions assert that the caller did the
right thing, as per attached.  This addresses the layering-violation
complaint, but might be more of a pain in the rear for developers.

Not really sure which way I like better.

            regards, tom lane

diff --git a/src/backend/access/gist/gistbuildbuffers.c b/src/backend/access/gist/gistbuildbuffers.c
index 4f2363e..1d008ec 100644
--- a/src/backend/access/gist/gistbuildbuffers.c
+++ b/src/backend/access/gist/gistbuildbuffers.c
@@ -17,6 +17,7 @@
 #include "access/genam.h"
 #include "access/gist_private.h"
 #include "catalog/index.h"
+#include "commands/tablespace.h"
 #include "miscadmin.h"
 #include "storage/buffile.h"
 #include "storage/bufmgr.h"
@@ -58,6 +59,8 @@ gistInitBuildBuffers(int pagesPerBuffer, int levelStep, int maxLevel)
      * Create a temporary file to hold buffer pages that are swapped out of
      * memory.
      */
+    PrepareTempTablespaces();
+
     gfbb->pfile = BufFileCreateTemp(false);
     gfbb->nFileBlocks = 0;

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index fdac985..a4463ba 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -1459,6 +1459,9 @@ PathNameDeleteTemporaryDir(const char *dirname)
  * outlive the transaction that created them, so this should be false -- but
  * if you need "somewhat" temporary storage, this might be useful. In either
  * case, the file is removed when the File is explicitly closed.
+ *
+ * Unless interXact is true, some caller function should have done
+ * PrepareTempTablespaces().
  */
 File
 OpenTemporaryFile(bool interXact)
@@ -1481,7 +1484,7 @@ OpenTemporaryFile(bool interXact)
      * force it into the database's default tablespace, so that it will not
      * pose a threat to possible tablespace drop attempts.
      */
-    if (numTempTableSpaces > 0 && !interXact)
+    if (!interXact)
     {
         Oid            tblspcOid = GetNextTempTableSpace();

@@ -2732,6 +2735,7 @@ GetTempTablespaces(Oid *tableSpaces, int numSpaces)
 Oid
 GetNextTempTableSpace(void)
 {
+    Assert(TempTablespacesAreSet());
     if (numTempTableSpaces > 0)
     {
         /* Advance nextTempTableSpace counter with wraparound */