Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commitcae1c78

Browse files
committed
Improve the situation for parallel query versus temp relations.
Transmit the leader's temp-namespace state to workers. This is importantbecause without it, the workers do not really have the same search pathas the leader. For example, there is no good reason (and no extant codeeither) to prevent a worker from executing a temp function that theleader created previously; but as things stood it would fail to find thetemp 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 leaderhad created it before starting the worker, which seems like the rightsemantics.Also, transmit the leader's BackendId to workers, and arrange for workersto use that when determining the physical file path of a temp relationbelonging to their session. While the original intent was to prevent suchaccesses entirely, there were a number of holes in that, notably in placeslike dbsize.c which assume they can safely access temp rels of othersessions anyway. We might as well get this right, as a small down paymenton someday allowing workers to access the leader's temp tables. (Withthis change, directly using "MyBackendId" as a relation or buffer backendID is deprecated; you should use BackendIdForTempRelations() instead.I left a couple of such uses alone though, as they're not going to bereachable in parallel workers until we do something about localbuf.c.)Move the thou-shalt-not-access-thy-leader's-temp-tables prohibition downinto localbuf.c, which is where it actually matters, instead of having itin relation_open(). This amounts to recognizing that access to temptables' catalog entries is perfectly safe in a worker, it's only the datain 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 aprohibition, that was not even close to being a bulletproof guard for it.(For example, any user-defined function executed in a parallel workermight have attempted such access.)
1 parent2410a25 commitcae1c78

File tree

12 files changed

+92
-72
lines changed

12 files changed

+92
-72
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,13 +1131,7 @@ relation_open(Oid relationId, LOCKMODE lockmode)
11311131

11321132
/* Make note that we've accessed a temporary relation */
11331133
if (RelationUsesLocalBuffers(r))
1134-
{
1135-
if (IsParallelWorker())
1136-
ereport(ERROR,
1137-
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
1138-
errmsg("cannot access temporary tables during a parallel operation")));
11391134
MyXactAccessedTempRel= true;
1140-
}
11411135

11421136
pgstat_initstats(r);
11431137

@@ -1183,13 +1177,7 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
11831177

11841178
/* Make note that we've accessed a temporary relation */
11851179
if (RelationUsesLocalBuffers(r))
1186-
{
1187-
if (IsParallelWorker())
1188-
ereport(ERROR,
1189-
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
1190-
errmsg("cannot access temporary tables during a parallel operation")));
11911180
MyXactAccessedTempRel= true;
1192-
}
11931181

11941182
pgstat_initstats(r);
11951183

‎src/backend/access/transam/parallel.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include"access/xact.h"
1818
#include"access/xlog.h"
1919
#include"access/parallel.h"
20+
#include"catalog/namespace.h"
2021
#include"commands/async.h"
2122
#include"libpq/libpq.h"
2223
#include"libpq/pqformat.h"
@@ -67,6 +68,8 @@ typedef struct FixedParallelState
6768
Oiddatabase_id;
6869
Oidauthenticated_user_id;
6970
Oidcurrent_user_id;
71+
Oidtemp_namespace_id;
72+
Oidtemp_toast_namespace_id;
7073
intsec_context;
7174
PGPROC*parallel_master_pgproc;
7275
pid_tparallel_master_pid;
@@ -288,6 +291,8 @@ InitializeParallelDSM(ParallelContext *pcxt)
288291
fps->database_id=MyDatabaseId;
289292
fps->authenticated_user_id=GetAuthenticatedUserId();
290293
GetUserIdAndSecContext(&fps->current_user_id,&fps->sec_context);
294+
GetTempNamespaceState(&fps->temp_namespace_id,
295+
&fps->temp_toast_namespace_id);
291296
fps->parallel_master_pgproc=MyProc;
292297
fps->parallel_master_pid=MyProcPid;
293298
fps->parallel_master_backend_id=MyBackendId;
@@ -1019,6 +1024,13 @@ ParallelWorkerMain(Datum main_arg)
10191024
/* Restore user ID and security context. */
10201025
SetUserIdAndSecContext(fps->current_user_id,fps->sec_context);
10211026

1027+
/* Restore temp-namespace state to ensure search path matches leader's. */
1028+
SetTempNamespaceState(fps->temp_namespace_id,
1029+
fps->temp_toast_namespace_id);
1030+
1031+
/* Set ParallelMasterBackendId so we know how to address temp relations. */
1032+
ParallelMasterBackendId=fps->parallel_master_backend_id;
1033+
10221034
/*
10231035
* We've initialized all of our state now; nothing should change
10241036
* hereafter.

‎src/backend/catalog/catalog.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence)
390390
switch (relpersistence)
391391
{
392392
caseRELPERSISTENCE_TEMP:
393-
backend=MyBackendId;
393+
backend=BackendIdForTempRelations();
394394
break;
395395
caseRELPERSISTENCE_UNLOGGED:
396396
caseRELPERSISTENCE_PERMANENT:

‎src/backend/catalog/namespace.c

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3073,6 +3073,51 @@ GetTempToastNamespace(void)
30733073
}
30743074

30753075

3076+
/*
3077+
* GetTempNamespaceState - fetch status of session's temporary namespace
3078+
*
3079+
* This is used for conveying state to a parallel worker, and is not meant
3080+
* for general-purpose access.
3081+
*/
3082+
void
3083+
GetTempNamespaceState(Oid*tempNamespaceId,Oid*tempToastNamespaceId)
3084+
{
3085+
/* Return namespace OIDs, or 0 if session has not created temp namespace */
3086+
*tempNamespaceId=myTempNamespace;
3087+
*tempToastNamespaceId=myTempToastNamespace;
3088+
}
3089+
3090+
/*
3091+
* SetTempNamespaceState - set status of session's temporary namespace
3092+
*
3093+
* This is used for conveying state to a parallel worker, and is not meant for
3094+
* general-purpose access. By transferring these namespace OIDs to workers,
3095+
* we ensure they will have the same notion of the search path as their leader
3096+
* does.
3097+
*/
3098+
void
3099+
SetTempNamespaceState(OidtempNamespaceId,OidtempToastNamespaceId)
3100+
{
3101+
/* Worker should not have created its own namespaces ... */
3102+
Assert(myTempNamespace==InvalidOid);
3103+
Assert(myTempToastNamespace==InvalidOid);
3104+
Assert(myTempNamespaceSubID==InvalidSubTransactionId);
3105+
3106+
/* Assign same namespace OIDs that leader has */
3107+
myTempNamespace=tempNamespaceId;
3108+
myTempToastNamespace=tempToastNamespaceId;
3109+
3110+
/*
3111+
* It's fine to leave myTempNamespaceSubID == InvalidSubTransactionId.
3112+
* Even if the namespace is new so far as the leader is concerned, it's
3113+
* not new to the worker, and we certainly wouldn't want the worker trying
3114+
* to destroy it.
3115+
*/
3116+
3117+
baseSearchPathValid= false;/* may need to rebuild list */
3118+
}
3119+
3120+
30763121
/*
30773122
* GetOverrideSearchPath - fetch current search path definition in form
30783123
* used by PushOverrideSearchPath.

‎src/backend/catalog/storage.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ RelationCreateStorage(RelFileNode rnode, char relpersistence)
8585
switch (relpersistence)
8686
{
8787
caseRELPERSISTENCE_TEMP:
88-
backend=MyBackendId;
88+
backend=BackendIdForTempRelations();
8989
needs_wal= false;
9090
break;
9191
caseRELPERSISTENCE_UNLOGGED:

‎src/backend/optimizer/util/clauses.c

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ static bool has_parallel_hazard_walker(Node *node,
115115
has_parallel_hazard_arg*context);
116116
staticboolparallel_too_dangerous(charproparallel,
117117
has_parallel_hazard_arg*context);
118-
staticbooltypeid_is_temp(Oidtypeid);
119118
staticboolcontain_nonstrict_functions_walker(Node*node,void*context);
120119
staticboolcontain_leaked_vars_walker(Node*node,void*context);
121120
staticRelidsfind_nonnullable_rels_walker(Node*node,booltop_level);
@@ -1409,49 +1408,6 @@ has_parallel_hazard_walker(Node *node, has_parallel_hazard_arg *context)
14091408
returnhas_parallel_hazard_walker((Node*)rinfo->clause,context);
14101409
}
14111410

1412-
/*
1413-
* It is an error for a parallel worker to touch a temporary table in any
1414-
* way, so we can't handle nodes whose type is the rowtype of such a
1415-
* table.
1416-
*/
1417-
if (!context->allow_restricted)
1418-
{
1419-
switch (nodeTag(node))
1420-
{
1421-
caseT_Var:
1422-
caseT_Const:
1423-
caseT_Param:
1424-
caseT_Aggref:
1425-
caseT_WindowFunc:
1426-
caseT_ArrayRef:
1427-
caseT_FuncExpr:
1428-
caseT_NamedArgExpr:
1429-
caseT_OpExpr:
1430-
caseT_DistinctExpr:
1431-
caseT_NullIfExpr:
1432-
caseT_FieldSelect:
1433-
caseT_FieldStore:
1434-
caseT_RelabelType:
1435-
caseT_CoerceViaIO:
1436-
caseT_ArrayCoerceExpr:
1437-
caseT_ConvertRowtypeExpr:
1438-
caseT_CaseExpr:
1439-
caseT_CaseTestExpr:
1440-
caseT_ArrayExpr:
1441-
caseT_RowExpr:
1442-
caseT_CoalesceExpr:
1443-
caseT_MinMaxExpr:
1444-
caseT_CoerceToDomain:
1445-
caseT_CoerceToDomainValue:
1446-
caseT_SetToDefault:
1447-
if (typeid_is_temp(exprType(node)))
1448-
return true;
1449-
break;
1450-
default:
1451-
break;
1452-
}
1453-
}
1454-
14551411
/*
14561412
* For each node that might potentially call a function, we need to
14571413
* 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)
15581514
returnproparallel!=PROPARALLEL_SAFE;
15591515
}
15601516

1561-
staticbool
1562-
typeid_is_temp(Oidtypeid)
1563-
{
1564-
Oidrelid=get_typ_typrelid(typeid);
1565-
1566-
if (!OidIsValid(relid))
1567-
return false;
1568-
1569-
return (get_rel_persistence(relid)==RELPERSISTENCE_TEMP);
1570-
}
1571-
15721517
/*****************************************************************************
15731518
*Check clauses for nonstrict functions
15741519
*****************************************************************************/

‎src/backend/storage/buffer/localbuf.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
#include"postgres.h"
1717

18+
#include"access/parallel.h"
1819
#include"catalog/catalog.h"
1920
#include"executor/instrument.h"
2021
#include"storage/buf_internals.h"
@@ -412,6 +413,19 @@ InitLocalBuffers(void)
412413
HASHCTLinfo;
413414
inti;
414415

416+
/*
417+
* Parallel workers can't access data in temporary tables, because they
418+
* have no visibility into the local buffers of their leader. This is a
419+
* convenient, low-cost place to provide a backstop check for that. Note
420+
* that we don't wish to prevent a parallel worker from accessing catalog
421+
* metadata about a temp table, so checks at higher levels would be
422+
* inappropriate.
423+
*/
424+
if (IsParallelWorker())
425+
ereport(ERROR,
426+
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
427+
errmsg("cannot access temporary tables during a parallel operation")));
428+
415429
/* Allocate and zero buffer headers and auxiliary arrays */
416430
LocalBufferDescriptors= (BufferDesc*)calloc(nbufs,sizeof(BufferDesc));
417431
LocalBufferBlockPointers= (Block*)calloc(nbufs,sizeof(Block));

‎src/backend/utils/adt/dbsize.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@ pg_relation_filepath(PG_FUNCTION_ARGS)
10041004
break;
10051005
caseRELPERSISTENCE_TEMP:
10061006
if (isTempOrTempToastNamespace(relform->relnamespace))
1007-
backend=MyBackendId;
1007+
backend=BackendIdForTempRelations();
10081008
else
10091009
{
10101010
/* Do it the hard way. */

‎src/backend/utils/cache/relcache.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,7 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
993993
caseRELPERSISTENCE_TEMP:
994994
if (isTempOrTempToastNamespace(relation->rd_rel->relnamespace))
995995
{
996-
relation->rd_backend=MyBackendId;
996+
relation->rd_backend=BackendIdForTempRelations();
997997
relation->rd_islocaltemp= true;
998998
}
999999
else
@@ -2970,7 +2970,7 @@ RelationBuildLocalRelation(const char *relname,
29702970
break;
29712971
caseRELPERSISTENCE_TEMP:
29722972
Assert(isTempOrTempToastNamespace(relnamespace));
2973-
rel->rd_backend=MyBackendId;
2973+
rel->rd_backend=BackendIdForTempRelations();
29742974
rel->rd_islocaltemp= true;
29752975
break;
29762976
default:

‎src/backend/utils/init/globals.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ charpostgres_exec_path[MAXPGPATH];/* full path to backend */
7171

7272
BackendIdMyBackendId=InvalidBackendId;
7373

74+
BackendIdParallelMasterBackendId=InvalidBackendId;
75+
7476
OidMyDatabaseId=InvalidOid;
7577

7678
OidMyDatabaseTableSpace=InvalidOid;

‎src/include/catalog/namespace.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ extern bool isAnyTempNamespace(Oid namespaceId);
125125
externboolisOtherTempNamespace(OidnamespaceId);
126126
externintGetTempNamespaceBackendId(OidnamespaceId);
127127
externOidGetTempToastNamespace(void);
128+
externvoidGetTempNamespaceState(Oid*tempNamespaceId,
129+
Oid*tempToastNamespaceId);
130+
externvoidSetTempNamespaceState(OidtempNamespaceId,
131+
OidtempToastNamespaceId);
128132
externvoidResetTempTableNamespace(void);
129133

130134
externOverrideSearchPath*GetOverrideSearchPath(MemoryContextcontext);

‎src/include/storage/backendid.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,14 @@ typedef int BackendId;/* unique currently active backend identifier */
2424

2525
externPGDLLIMPORTBackendIdMyBackendId;/* backend id of this backend */
2626

27+
/* backend id of our parallel session leader, or InvalidBackendId if none */
28+
externPGDLLIMPORTBackendIdParallelMasterBackendId;
29+
30+
/*
31+
* The BackendId to use for our session's temp relations is normally our own,
32+
* but parallel workers should use their leader's ID.
33+
*/
34+
#defineBackendIdForTempRelations() \
35+
(ParallelMasterBackendId == InvalidBackendId ? MyBackendId : ParallelMasterBackendId)
36+
2737
#endif/* BACKENDID_H */

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp