Improve the situation for parallel query versus temp relations.

Transmit the leader's temp-namespace state to workers.  This is important
because without it, the workers do not really have the same search path
as the leader.  For example, there is no good reason (and no extant code
either) to prevent a worker from executing a temp function that the
leader created previously; but as things stood it would fail to find the
temp function, and then either fail or execute the wrong function entirely.

We still prohibit a worker from creating a temp namespace on its own.
In effect, a worker can only see the session's temp namespace if the leader
had created it before starting the worker, which seems like the right
semantics.

Also, transmit the leader's BackendId to workers, and arrange for workers
to use that when determining the physical file path of a temp relation
belonging to their session.  While the original intent was to prevent such
accesses entirely, there were a number of holes in that, notably in places
like dbsize.c which assume they can safely access temp rels of other
sessions anyway.  We might as well get this right, as a small down payment
on someday allowing workers to access the leader's temp tables.  (With
this change, directly using "MyBackendId" as a relation or buffer backend
ID is deprecated; you should use BackendIdForTempRelations() instead.
I left a couple of such uses alone though, as they're not going to be
reachable in parallel workers until we do something about localbuf.c.)

Move the thou-shalt-not-access-thy-leader's-temp-tables prohibition down
into localbuf.c, which is where it actually matters, instead of having it
in relation_open().  This amounts to recognizing that access to temp
tables' catalog entries is perfectly safe in a worker, it's only the data
in local buffers that is problematic.

Having done all that, we can get rid of the test in has_parallel_hazard()
that says that use of a temp table's rowtype is unsafe in parallel workers.
That test was unduly expensive, and if we really did need such a
prohibition, that was not even close to being a bulletproof guard for it.
(For example, any user-defined function executed in a parallel worker
might have attempted such access.)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 6db624109797fc409973ad3af670a0882982a0c7..0b3332ecf5dd0211de5ade4e601801ab0a064bfb 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1131,13 +1131,7 @@ relation_open(Oid relationId, LOCKMODE lockmode)
 
    /* Make note that we've accessed a temporary relation */
    if (RelationUsesLocalBuffers(r))
-   {
-       if (IsParallelWorker())
-           ereport(ERROR,
-                   (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
-                    errmsg("cannot access temporary tables during a parallel operation")));
        MyXactAccessedTempRel = true;
-   }
 
    pgstat_initstats(r);
 
@@ -1183,13 +1177,7 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
 
    /* Make note that we've accessed a temporary relation */
    if (RelationUsesLocalBuffers(r))
-   {
-       if (IsParallelWorker())
-           ereport(ERROR,
-                   (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
-                    errmsg("cannot access temporary tables during a parallel operation")));
        MyXactAccessedTempRel = true;
-   }
 
    pgstat_initstats(r);
 

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 74a483e0fd9f4efac7874c46e7c23406bc1e3ee5..ab5ef2573cffd6184a12cf94ed09e2d2fe00bdf7 100644 (file)
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -17,6 +17,7 @@
 #include "access/xact.h"
 #include "access/xlog.h"
 #include "access/parallel.h"
+#include "catalog/namespace.h"
 #include "commands/async.h"
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
@@ -67,6 +68,8 @@ typedef struct FixedParallelState
    Oid         database_id;
    Oid         authenticated_user_id;
    Oid         current_user_id;
+   Oid         temp_namespace_id;
+   Oid         temp_toast_namespace_id;
    int         sec_context;
    PGPROC     *parallel_master_pgproc;
    pid_t       parallel_master_pid;
@@ -288,6 +291,8 @@ InitializeParallelDSM(ParallelContext *pcxt)
    fps->database_id = MyDatabaseId;
    fps->authenticated_user_id = GetAuthenticatedUserId();
    GetUserIdAndSecContext(&fps->current_user_id, &fps->sec_context);
+   GetTempNamespaceState(&fps->temp_namespace_id,
+                         &fps->temp_toast_namespace_id);
    fps->parallel_master_pgproc = MyProc;
    fps->parallel_master_pid = MyProcPid;
    fps->parallel_master_backend_id = MyBackendId;
@@ -1019,6 +1024,13 @@ ParallelWorkerMain(Datum main_arg)
    /* Restore user ID and security context. */
    SetUserIdAndSecContext(fps->current_user_id, fps->sec_context);
 
+   /* Restore temp-namespace state to ensure search path matches leader's. */
+   SetTempNamespaceState(fps->temp_namespace_id,
+                         fps->temp_toast_namespace_id);
+
+   /* Set ParallelMasterBackendId so we know how to address temp relations. */
+   ParallelMasterBackendId = fps->parallel_master_backend_id;
+
    /*
     * We've initialized all of our state now; nothing should change
     * hereafter.

diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index d1cf45bef47b1b285a445bb09194edbdbbd14194..1baaa0bb8988cb348e03dcd38f64706a47e385d2 100644 (file)
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -390,7 +390,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence)
    switch (relpersistence)
    {
        case RELPERSISTENCE_TEMP:
-           backend = MyBackendId;
+           backend = BackendIdForTempRelations();
            break;
        case RELPERSISTENCE_UNLOGGED:
        case RELPERSISTENCE_PERMANENT:

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index a1aba8ee556bf810214906d73d3b181215b0987d..8fd4c3136bc44726c4d848e1935be9977a30c6e9 100644 (file)
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -3073,6 +3073,51 @@ GetTempToastNamespace(void)
 }
 
 
+/*
+ * GetTempNamespaceState - fetch status of session's temporary namespace
+ *
+ * This is used for conveying state to a parallel worker, and is not meant
+ * for general-purpose access.
+ */
+void
+GetTempNamespaceState(Oid *tempNamespaceId, Oid *tempToastNamespaceId)
+{
+   /* Return namespace OIDs, or 0 if session has not created temp namespace */
+   *tempNamespaceId = myTempNamespace;
+   *tempToastNamespaceId = myTempToastNamespace;
+}
+
+/*
+ * SetTempNamespaceState - set status of session's temporary namespace
+ *
+ * This is used for conveying state to a parallel worker, and is not meant for
+ * general-purpose access.  By transferring these namespace OIDs to workers,
+ * we ensure they will have the same notion of the search path as their leader
+ * does.
+ */
+void
+SetTempNamespaceState(Oid tempNamespaceId, Oid tempToastNamespaceId)
+{
+   /* Worker should not have created its own namespaces ... */
+   Assert(myTempNamespace == InvalidOid);
+   Assert(myTempToastNamespace == InvalidOid);
+   Assert(myTempNamespaceSubID == InvalidSubTransactionId);
+
+   /* Assign same namespace OIDs that leader has */
+   myTempNamespace = tempNamespaceId;
+   myTempToastNamespace = tempToastNamespaceId;
+
+   /*
+    * It's fine to leave myTempNamespaceSubID == InvalidSubTransactionId.
+    * Even if the namespace is new so far as the leader is concerned, it's
+    * not new to the worker, and we certainly wouldn't want the worker trying
+    * to destroy it.
+    */
+
+   baseSearchPathValid = false;    /* may need to rebuild list */
+}
+
+
 /*
  * GetOverrideSearchPath - fetch current search path definition in form
  * used by PushOverrideSearchPath.

diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index fe68c998e8de130c0fb446e22a4090f5c7aca66b..67f1906326489afe89c63ecc521913da1dd24a52 100644 (file)
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -85,7 +85,7 @@ RelationCreateStorage(RelFileNode rnode, char relpersistence)
    switch (relpersistence)
    {
        case RELPERSISTENCE_TEMP:
-           backend = MyBackendId;
+           backend = BackendIdForTempRelations();
            needs_wal = false;
            break;
        case RELPERSISTENCE_UNLOGGED:

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index e7909eb5d5958532f5e8b5b82544ec4ef227d218..dc814a2b3ac8eb44ed62a4837592c42a449461a5 100644 (file)
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -115,7 +115,6 @@ static bool has_parallel_hazard_walker(Node *node,
                           has_parallel_hazard_arg *context);
 static bool parallel_too_dangerous(char proparallel,
                       has_parallel_hazard_arg *context);
-static bool typeid_is_temp(Oid typeid);
 static bool contain_nonstrict_functions_walker(Node *node, void *context);
 static bool contain_leaked_vars_walker(Node *node, void *context);
 static Relids find_nonnullable_rels_walker(Node *node, bool top_level);
@@ -1409,49 +1408,6 @@ has_parallel_hazard_walker(Node *node, has_parallel_hazard_arg *context)
        return has_parallel_hazard_walker((Node *) rinfo->clause, context);
    }
 
-   /*
-    * It is an error for a parallel worker to touch a temporary table in any
-    * way, so we can't handle nodes whose type is the rowtype of such a
-    * table.
-    */
-   if (!context->allow_restricted)
-   {
-       switch (nodeTag(node))
-       {
-           case T_Var:
-           case T_Const:
-           case T_Param:
-           case T_Aggref:
-           case T_WindowFunc:
-           case T_ArrayRef:
-           case T_FuncExpr:
-           case T_NamedArgExpr:
-           case T_OpExpr:
-           case T_DistinctExpr:
-           case T_NullIfExpr:
-           case T_FieldSelect:
-           case T_FieldStore:
-           case T_RelabelType:
-           case T_CoerceViaIO:
-           case T_ArrayCoerceExpr:
-           case T_ConvertRowtypeExpr:
-           case T_CaseExpr:
-           case T_CaseTestExpr:
-           case T_ArrayExpr:
-           case T_RowExpr:
-           case T_CoalesceExpr:
-           case T_MinMaxExpr:
-           case T_CoerceToDomain:
-           case T_CoerceToDomainValue:
-           case T_SetToDefault:
-               if (typeid_is_temp(exprType(node)))
-                   return true;
-               break;
-           default:
-               break;
-       }
-   }
-
    /*
     * For each node that might potentially call a function, we need to
     * examine the pg_proc.proparallel marking for that function to see
@@ -1558,17 +1514,6 @@ parallel_too_dangerous(char proparallel, has_parallel_hazard_arg *context)
        return proparallel != PROPARALLEL_SAFE;
 }
 
-static bool
-typeid_is_temp(Oid typeid)
-{
-   Oid         relid = get_typ_typrelid(typeid);
-
-   if (!OidIsValid(relid))
-       return false;
-
-   return (get_rel_persistence(relid) == RELPERSISTENCE_TEMP);
-}
-
 /*****************************************************************************
  *     Check clauses for nonstrict functions
  *****************************************************************************/

diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 201ce2668fd4fe1d1e40431f12ced6574b9523d8..53981794b98c7b803a5cbe8c8ad39d9b90008f8c 100644 (file)
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -15,6 +15,7 @@
  */
 #include "postgres.h"
 
+#include "access/parallel.h"
 #include "catalog/catalog.h"
 #include "executor/instrument.h"
 #include "storage/buf_internals.h"
@@ -412,6 +413,19 @@ InitLocalBuffers(void)
    HASHCTL     info;
    int         i;
 
+   /*
+    * Parallel workers can't access data in temporary tables, because they
+    * have no visibility into the local buffers of their leader.  This is a
+    * convenient, low-cost place to provide a backstop check for that.  Note
+    * that we don't wish to prevent a parallel worker from accessing catalog
+    * metadata about a temp table, so checks at higher levels would be
+    * inappropriate.
+    */
+   if (IsParallelWorker())
+       ereport(ERROR,
+               (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
+                errmsg("cannot access temporary tables during a parallel operation")));
+
    /* Allocate and zero buffer headers and auxiliary arrays */
    LocalBufferDescriptors = (BufferDesc *) calloc(nbufs, sizeof(BufferDesc));
    LocalBufferBlockPointers = (Block *) calloc(nbufs, sizeof(Block));

diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 4ec2fed2b354232d3d66cb830f0e21507c40d1a7..0776f3bf6cf6c0b825bc80c7a437691f72e6839a 100644 (file)
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -1004,7 +1004,7 @@ pg_relation_filepath(PG_FUNCTION_ARGS)
            break;
        case RELPERSISTENCE_TEMP:
            if (isTempOrTempToastNamespace(relform->relnamespace))
-               backend = MyBackendId;
+               backend = BackendIdForTempRelations();
            else
            {
                /* Do it the hard way. */

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index afb6c8772da1627187b795ee83701a3736763e61..70a651a8fc322115cef14010d63bad4cf0876db8 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -993,7 +993,7 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
        case RELPERSISTENCE_TEMP:
            if (isTempOrTempToastNamespace(relation->rd_rel->relnamespace))
            {
-               relation->rd_backend = MyBackendId;
+               relation->rd_backend = BackendIdForTempRelations();
                relation->rd_islocaltemp = true;
            }
            else
@@ -2970,7 +2970,7 @@ RelationBuildLocalRelation(const char *relname,
            break;
        case RELPERSISTENCE_TEMP:
            Assert(isTempOrTempToastNamespace(relnamespace));
-           rel->rd_backend = MyBackendId;
+           rel->rd_backend = BackendIdForTempRelations();
            rel->rd_islocaltemp = true;
            break;
        default:

diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 5a7783b981e94060a3971831a21104b90e94bbfb..f23208353c340137700e19fbc37804136d3fdc0e 100644 (file)
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -71,6 +71,8 @@ char      postgres_exec_path[MAXPGPATH];      /* full path to backend */
 
 BackendId  MyBackendId = InvalidBackendId;
 
+BackendId  ParallelMasterBackendId = InvalidBackendId;
+
 Oid            MyDatabaseId = InvalidOid;
 
 Oid            MyDatabaseTableSpace = InvalidOid;

diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index 2ccb3a7a03ad056669b4a070ad0e78191b0a96c7..eee94d862267440c89c7d296805c14bc5b41a042 100644 (file)
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -125,6 +125,10 @@ extern bool isAnyTempNamespace(Oid namespaceId);
 extern bool isOtherTempNamespace(Oid namespaceId);
 extern int GetTempNamespaceBackendId(Oid namespaceId);
 extern Oid GetTempToastNamespace(void);
+extern void GetTempNamespaceState(Oid *tempNamespaceId,
+                     Oid *tempToastNamespaceId);
+extern void SetTempNamespaceState(Oid tempNamespaceId,
+                     Oid tempToastNamespaceId);
 extern void ResetTempTableNamespace(void);
 
 extern OverrideSearchPath *GetOverrideSearchPath(MemoryContext context);

diff --git a/src/include/storage/backendid.h b/src/include/storage/backendid.h
index 7d9676f5348be0435b6394322a7a5e7e394cdffe..dec8a97271506d9724b5d3923ac71dc4b78ece0f 100644 (file)
--- a/src/include/storage/backendid.h
+++ b/src/include/storage/backendid.h
@@ -24,4 +24,14 @@ typedef int BackendId;           /* unique currently active backend identifier */
 
 extern PGDLLIMPORT BackendId MyBackendId;      /* backend id of this backend */
 
+/* backend id of our parallel session leader, or InvalidBackendId if none */
+extern PGDLLIMPORT BackendId ParallelMasterBackendId;
+
+/*
+ * The BackendId to use for our session's temp relations is normally our own,
+ * but parallel workers should use their leader's ID.
+ */
+#define BackendIdForTempRelations() \
+   (ParallelMasterBackendId == InvalidBackendId ? MyBackendId : ParallelMasterBackendId)
+
 #endif   /* BACKENDID_H */