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

Commitf23bdda

Browse files
committed
Fix LOCK TABLE to eliminate the race condition that could make it give weird
errors when tables are concurrently dropped. To do this we must take lockon each relation before we check its privileges. The old code was tryingto do that the other way around, which is a bit pointless when there are lotsof other commands that lock relations before checking privileges. I did keepit checking each relation's privilege before locking the next relation, whichis a detail that ALTER TABLE isn't too picky about.
1 parentd4a363c commitf23bdda

File tree

3 files changed

+114
-92
lines changed

3 files changed

+114
-92
lines changed

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

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.274 2009/01/20 18:59:36 heikki Exp $
11+
* $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.275 2009/05/12 16:43:32 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -947,56 +947,6 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
947947
returnr;
948948
}
949949

950-
/* ----------------
951-
*relation_open_nowait - open but don't wait for lock
952-
*
953-
*Same as relation_open, except throw an error instead of waiting
954-
*when the requested lock is not immediately obtainable.
955-
* ----------------
956-
*/
957-
Relation
958-
relation_open_nowait(OidrelationId,LOCKMODElockmode)
959-
{
960-
Relationr;
961-
962-
Assert(lockmode >=NoLock&&lockmode<MAX_LOCKMODES);
963-
964-
/* Get the lock before trying to open the relcache entry */
965-
if (lockmode!=NoLock)
966-
{
967-
if (!ConditionalLockRelationOid(relationId,lockmode))
968-
{
969-
/* try to throw error by name; relation could be deleted... */
970-
char*relname=get_rel_name(relationId);
971-
972-
if (relname)
973-
ereport(ERROR,
974-
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
975-
errmsg("could not obtain lock on relation \"%s\"",
976-
relname)));
977-
else
978-
ereport(ERROR,
979-
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
980-
errmsg("could not obtain lock on relation with OID %u",
981-
relationId)));
982-
}
983-
}
984-
985-
/* The relcache does all the real work... */
986-
r=RelationIdGetRelation(relationId);
987-
988-
if (!RelationIsValid(r))
989-
elog(ERROR,"could not open relation with OID %u",relationId);
990-
991-
/* Make note that we've accessed a temporary relation */
992-
if (r->rd_istemp)
993-
MyXactAccessedTempRel= true;
994-
995-
pgstat_initstats(r);
996-
997-
returnr;
998-
}
999-
1000950
/* ----------------
1001951
*relation_openrv - open any relation specified by a RangeVar
1002952
*

‎src/backend/commands/lockcmds.c

Lines changed: 112 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
/*-------------------------------------------------------------------------
22
*
33
* lockcmds.c
4-
*Lock command support code
4+
*LOCK command support code
55
*
66
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.23 2009/05/1203:11:01 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.24 2009/05/1216:43:32 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -20,9 +20,12 @@
2020
#include"commands/lockcmds.h"
2121
#include"miscadmin.h"
2222
#include"parser/parse_clause.h"
23+
#include"storage/lmgr.h"
2324
#include"utils/acl.h"
2425
#include"utils/lsyscache.h"
25-
#include"utils/rel.h"
26+
27+
staticvoidLockTableRecurse(Oidreloid,RangeVar*rv,
28+
LOCKMODElockmode,boolnowait,boolrecurse);
2629

2730

2831
/*
@@ -34,57 +37,127 @@ LockTableCommand(LockStmt *lockstmt)
3437
ListCell*p;
3538

3639
/*
37-
* Iterate over the list and open, lock, and close the relations one at a
38-
* time
40+
* Iterate over the list and process the named relations one at a time
3941
*/
40-
4142
foreach(p,lockstmt->relations)
4243
{
43-
RangeVar*relation=lfirst(p);
44-
Oidreloid;
44+
RangeVar*relation= (RangeVar*)lfirst(p);
4545
boolrecurse=interpretInhOption(relation->inhOpt);
46-
List*children_and_self;
47-
ListCell*child;
46+
Oidreloid;
4847

4948
reloid=RangeVarGetRelid(relation, false);
5049

51-
/* XXX NoLock here is not really a good idea */
52-
if (recurse)
53-
children_and_self=find_all_inheritors(reloid,NoLock);
54-
else
55-
children_and_self=list_make1_oid(reloid);
50+
LockTableRecurse(reloid,relation,
51+
lockstmt->mode,lockstmt->nowait,recurse);
52+
}
53+
}
5654

57-
foreach(child,children_and_self)
55+
/*
56+
* Apply LOCK TABLE recursively over an inheritance tree
57+
*
58+
* At top level, "rv" is the original command argument; we use it to throw
59+
* an appropriate error message if the relation isn't there. Below top level,
60+
* "rv" is NULL and we should just silently ignore any dropped child rel.
61+
*/
62+
staticvoid
63+
LockTableRecurse(Oidreloid,RangeVar*rv,
64+
LOCKMODElockmode,boolnowait,boolrecurse)
65+
{
66+
Relationrel;
67+
AclResultaclresult;
68+
69+
/*
70+
* Acquire the lock. We must do this first to protect against
71+
* concurrent drops. Note that a lock against an already-dropped
72+
* relation's OID won't fail.
73+
*/
74+
if (nowait)
75+
{
76+
if (!ConditionalLockRelationOid(reloid,lockmode))
5877
{
59-
Oidchildreloid=lfirst_oid(child);
60-
Relationrel;
61-
AclResultaclresult;
62-
63-
/* We don't want to open the relation until we've checked privilege. */
64-
if (lockstmt->mode==AccessShareLock)
65-
aclresult=pg_class_aclcheck(childreloid,GetUserId(),
66-
ACL_SELECT);
78+
/* try to throw error by name; relation could be deleted... */
79+
char*relname=rv ?rv->relname :get_rel_name(reloid);
80+
81+
if (relname)
82+
ereport(ERROR,
83+
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
84+
errmsg("could not obtain lock on relation \"%s\"",
85+
relname)));
6786
else
68-
aclresult=pg_class_aclcheck(childreloid,GetUserId(),
69-
ACL_UPDATE |ACL_DELETE |ACL_TRUNCATE);
87+
ereport(ERROR,
88+
(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
89+
errmsg("could not obtain lock on relation with OID %u",
90+
reloid)));
91+
}
92+
}
93+
else
94+
LockRelationOid(reloid,lockmode);
7095

71-
if (aclresult!=ACLCHECK_OK)
72-
aclcheck_error(aclresult,ACL_KIND_CLASS,
73-
get_rel_name(childreloid));
96+
/*
97+
* Now that we have the lock, check to see if the relation really exists
98+
* or not.
99+
*/
100+
rel=try_relation_open(reloid,NoLock);
74101

75-
if (lockstmt->nowait)
76-
rel=relation_open_nowait(childreloid,lockstmt->mode);
77-
else
78-
rel=relation_open(childreloid,lockstmt->mode);
102+
if (!rel)
103+
{
104+
/* Release useless lock */
105+
UnlockRelationOid(reloid,lockmode);
79106

80-
/* Currently, we only allow plain tables to be locked */
81-
if (rel->rd_rel->relkind!=RELKIND_RELATION)
107+
/* At top level, throw error; otherwise, ignore this child rel */
108+
if (rv)
109+
{
110+
if (rv->schemaname)
82111
ereport(ERROR,
83-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
84-
errmsg("\"%s\" is not a table",
85-
get_rel_name(childreloid))));
112+
(errcode(ERRCODE_UNDEFINED_TABLE),
113+
errmsg("relation \"%s.%s\" does not exist",
114+
rv->schemaname,rv->relname)));
115+
else
116+
ereport(ERROR,
117+
(errcode(ERRCODE_UNDEFINED_TABLE),
118+
errmsg("relation \"%s\" does not exist",
119+
rv->relname)));
120+
}
121+
122+
return;
123+
}
86124

87-
relation_close(rel,NoLock);/* close rel, keep lock */
125+
/* Verify adequate privilege */
126+
if (lockmode==AccessShareLock)
127+
aclresult=pg_class_aclcheck(reloid,GetUserId(),
128+
ACL_SELECT);
129+
else
130+
aclresult=pg_class_aclcheck(reloid,GetUserId(),
131+
ACL_UPDATE |ACL_DELETE |ACL_TRUNCATE);
132+
if (aclresult!=ACLCHECK_OK)
133+
aclcheck_error(aclresult,ACL_KIND_CLASS,
134+
RelationGetRelationName(rel));
135+
136+
/* Currently, we only allow plain tables to be locked */
137+
if (rel->rd_rel->relkind!=RELKIND_RELATION)
138+
ereport(ERROR,
139+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
140+
errmsg("\"%s\" is not a table",
141+
RelationGetRelationName(rel))));
142+
143+
/*
144+
* If requested, recurse to children. We use find_inheritance_children
145+
* not find_all_inheritors to avoid taking locks far in advance of
146+
* checking privileges. This means we'll visit multiply-inheriting
147+
* children more than once, but that's no problem.
148+
*/
149+
if (recurse)
150+
{
151+
List*children=find_inheritance_children(reloid,NoLock);
152+
ListCell*lc;
153+
154+
foreach(lc,children)
155+
{
156+
Oidchildreloid=lfirst_oid(lc);
157+
158+
LockTableRecurse(childreloid,NULL,lockmode,nowait,recurse);
88159
}
89160
}
161+
162+
relation_close(rel,NoLock);/* close rel, keep lock */
90163
}

‎src/include/access/heapam.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/access/heapam.h,v 1.141 2009/01/01 17:23:56 momjian Exp $
10+
* $PostgreSQL: pgsql/src/include/access/heapam.h,v 1.142 2009/05/12 16:43:32 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -49,7 +49,6 @@ typedef enum
4949
/* in heap/heapam.c */
5050
externRelationrelation_open(OidrelationId,LOCKMODElockmode);
5151
externRelationtry_relation_open(OidrelationId,LOCKMODElockmode);
52-
externRelationrelation_open_nowait(OidrelationId,LOCKMODElockmode);
5352
externRelationrelation_openrv(constRangeVar*relation,LOCKMODElockmode);
5453
externRelationtry_relation_openrv(constRangeVar*relation,LOCKMODElockmode);
5554
externvoidrelation_close(Relationrelation,LOCKMODElockmode);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp