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

Commitd4a363c

Browse files
committed
Modify find_inheritance_children() and find_all_inheritors() to add the
ability to lock relations as they scan pg_inherits, and to ignore anyrelations that have disappeared by the time we get lock on them. Thismakes uses of these functions safe against concurrent DROP operationson child tables: we will effectively ignore any just-dropped child,rather than possibly throwing an error as in recent bug report fromThomas Johansson (and similar past complaints). The behavior shouldnot change otherwise, since the code was acquiring those same locksanyway, just a little bit later.An exception is LockTableCommand(), which is still behaving unsafely;but that seems to require some more discussion before we change it.
1 parent0ada559 commitd4a363c

File tree

7 files changed

+135
-68
lines changed

7 files changed

+135
-68
lines changed

‎src/backend/catalog/pg_inherits.c‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $PostgreSQL: pgsql/src/backend/catalog/pg_inherits.c,v 1.1 2009/05/1200:56:05 tgl Exp $
16+
* $PostgreSQL: pgsql/src/backend/catalog/pg_inherits.c,v 1.2 2009/05/1203:11:01 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -22,7 +22,9 @@
2222
#include"access/heapam.h"
2323
#include"catalog/pg_class.h"
2424
#include"catalog/pg_inherits.h"
25+
#include"catalog/pg_inherits_fn.h"
2526
#include"parser/parse_type.h"
27+
#include"storage/lmgr.h"
2628
#include"utils/fmgroids.h"
2729
#include"utils/syscache.h"
2830
#include"utils/tqual.h"
@@ -33,9 +35,14 @@
3335
*
3436
* Returns a list containing the OIDs of all relations which
3537
* inherit *directly* from the relation with OID 'parentrelId'.
38+
*
39+
* The specified lock type is acquired on each child relation (but not on the
40+
* given rel; caller should already have locked it). If lockmode is NoLock
41+
* then no locks are acquired, but caller must beware of race conditions
42+
* against possible DROPs of child relations.
3643
*/
3744
List*
38-
find_inheritance_children(OidparentrelId)
45+
find_inheritance_children(OidparentrelId,LOCKMODElockmode)
3946
{
4047
List*list=NIL;
4148
Relationrelation;
@@ -63,13 +70,38 @@ find_inheritance_children(Oid parentrelId)
6370
BTEqualStrategyNumber,F_OIDEQ,
6471
ObjectIdGetDatum(parentrelId));
6572
scan=heap_beginscan(relation,SnapshotNow,1,key);
73+
6674
while ((inheritsTuple=heap_getnext(scan,ForwardScanDirection))!=NULL)
6775
{
6876
inhrelid= ((Form_pg_inherits)GETSTRUCT(inheritsTuple))->inhrelid;
77+
78+
if (lockmode!=NoLock)
79+
{
80+
/* Get the lock to synchronize against concurrent drop */
81+
LockRelationOid(inhrelid,lockmode);
82+
83+
/*
84+
* Now that we have the lock, double-check to see if the relation
85+
* really exists or not. If not, assume it was dropped while
86+
* we waited to acquire lock, and ignore it.
87+
*/
88+
if (!SearchSysCacheExists(RELOID,
89+
ObjectIdGetDatum(inhrelid),
90+
0,0,0))
91+
{
92+
/* Release useless lock */
93+
UnlockRelationOid(inhrelid,lockmode);
94+
/* And ignore this relation */
95+
continue;
96+
}
97+
}
98+
6999
list=lappend_oid(list,inhrelid);
70100
}
101+
71102
heap_endscan(scan);
72103
heap_close(relation,AccessShareLock);
104+
73105
returnlist;
74106
}
75107

@@ -78,9 +110,14 @@ find_inheritance_children(Oid parentrelId)
78110
* find_all_inheritors -
79111
*Returns a list of relation OIDs including the given rel plus
80112
*all relations that inherit from it, directly or indirectly.
113+
*
114+
* The specified lock type is acquired on all child relations (but not on the
115+
* given rel; caller should already have locked it). If lockmode is NoLock
116+
* then no locks are acquired, but caller must beware of race conditions
117+
* against possible DROPs of child relations.
81118
*/
82119
List*
83-
find_all_inheritors(OidparentrelId)
120+
find_all_inheritors(OidparentrelId,LOCKMODElockmode)
84121
{
85122
List*rels_list;
86123
ListCell*l;
@@ -100,7 +137,7 @@ find_all_inheritors(Oid parentrelId)
100137
List*currentchildren;
101138

102139
/* Get the direct children of this rel */
103-
currentchildren=find_inheritance_children(currentrel);
140+
currentchildren=find_inheritance_children(currentrel,lockmode);
104141

105142
/*
106143
* Add to the queue only those children not already seen. This avoids

‎src/backend/commands/lockcmds.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.22 2009/05/1200:56:05 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.23 2009/05/1203:11:01 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515
#include"postgres.h"
1616

1717
#include"access/heapam.h"
1818
#include"catalog/namespace.h"
19-
#include"catalog/pg_inherits.h"
19+
#include"catalog/pg_inherits_fn.h"
2020
#include"commands/lockcmds.h"
2121
#include"miscadmin.h"
2222
#include"parser/parse_clause.h"
@@ -48,8 +48,9 @@ LockTableCommand(LockStmt *lockstmt)
4848

4949
reloid=RangeVarGetRelid(relation, false);
5050

51+
/* XXX NoLock here is not really a good idea */
5152
if (recurse)
52-
children_and_self=find_all_inheritors(reloid);
53+
children_and_self=find_all_inheritors(reloid,NoLock);
5354
else
5455
children_and_self=list_make1_oid(reloid);
5556

‎src/backend/commands/tablecmds.c‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.283 2009/05/1200:56:05 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.284 2009/05/1203:11:01 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -29,6 +29,7 @@
2929
#include"catalog/pg_constraint.h"
3030
#include"catalog/pg_depend.h"
3131
#include"catalog/pg_inherits.h"
32+
#include"catalog/pg_inherits_fn.h"
3233
#include"catalog/pg_namespace.h"
3334
#include"catalog/pg_opclass.h"
3435
#include"catalog/pg_tablespace.h"
@@ -796,7 +797,7 @@ ExecuteTruncate(TruncateStmt *stmt)
796797
ListCell*child;
797798
List*children;
798799

799-
children=find_all_inheritors(myrelid);
800+
children=find_all_inheritors(myrelid,AccessExclusiveLock);
800801

801802
foreach(child,children)
802803
{
@@ -805,7 +806,8 @@ ExecuteTruncate(TruncateStmt *stmt)
805806
if (list_member_oid(relids,childrelid))
806807
continue;
807808

808-
rel=heap_open(childrelid,AccessExclusiveLock);
809+
/* find_all_inheritors already got lock */
810+
rel=heap_open(childrelid,NoLock);
809811
truncate_check_rel(rel);
810812
rels=lappend(rels,rel);
811813
relids=lappend_oid(relids,childrelid);
@@ -1871,7 +1873,7 @@ renameatt(Oid myrelid,
18711873
ListCell*child;
18721874
List*children;
18731875

1874-
children=find_all_inheritors(myrelid);
1876+
children=find_all_inheritors(myrelid,AccessExclusiveLock);
18751877

18761878
/*
18771879
* find_all_inheritors does the recursive search of the inheritance
@@ -1895,7 +1897,7 @@ renameatt(Oid myrelid,
18951897
* tables; else the rename would put them out of step.
18961898
*/
18971899
if (!recursing&&
1898-
find_inheritance_children(myrelid)!=NIL)
1900+
find_inheritance_children(myrelid,NoLock)!=NIL)
18991901
ereport(ERROR,
19001902
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
19011903
errmsg("inherited column \"%s\" must be renamed in child tables too",
@@ -3289,7 +3291,7 @@ ATSimpleRecursion(List **wqueue, Relation rel,
32893291
ListCell*child;
32903292
List*children;
32913293

3292-
children=find_all_inheritors(relid);
3294+
children=find_all_inheritors(relid,AccessExclusiveLock);
32933295

32943296
/*
32953297
* find_all_inheritors does the recursive search of the inheritance
@@ -3303,7 +3305,8 @@ ATSimpleRecursion(List **wqueue, Relation rel,
33033305

33043306
if (childrelid==relid)
33053307
continue;
3306-
childrel=relation_open(childrelid,AccessExclusiveLock);
3308+
/* find_all_inheritors already got lock */
3309+
childrel=relation_open(childrelid,NoLock);
33073310
CheckTableNotInUse(childrel,"ALTER TABLE");
33083311
ATPrepCmd(wqueue,childrel,cmd, false, true);
33093312
relation_close(childrel,NoLock);
@@ -3327,14 +3330,15 @@ ATOneLevelRecursion(List **wqueue, Relation rel,
33273330
ListCell*child;
33283331
List*children;
33293332

3330-
children=find_inheritance_children(relid);
3333+
children=find_inheritance_children(relid,AccessExclusiveLock);
33313334

33323335
foreach(child,children)
33333336
{
33343337
Oidchildrelid=lfirst_oid(child);
33353338
Relationchildrel;
33363339

3337-
childrel=relation_open(childrelid,AccessExclusiveLock);
3340+
/* find_inheritance_children already got lock */
3341+
childrel=relation_open(childrelid,NoLock);
33383342
CheckTableNotInUse(childrel,"ALTER TABLE");
33393343
ATPrepCmd(wqueue,childrel,cmd, true, true);
33403344
relation_close(childrel,NoLock);
@@ -3480,7 +3484,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse,
34803484
* If we are told not to recurse, there had better not be any child
34813485
* tables; else the addition would put them out of step.
34823486
*/
3483-
if (find_inheritance_children(RelationGetRelid(rel))!=NIL)
3487+
if (find_inheritance_children(RelationGetRelid(rel),NoLock)!=NIL)
34843488
ereport(ERROR,
34853489
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
34863490
errmsg("column must be added to child tables too")));
@@ -4199,7 +4203,8 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
41994203
* routines, we have to do this one level of recursion at a time; we can't
42004204
* use find_all_inheritors to do it in one pass.
42014205
*/
4202-
children=find_inheritance_children(RelationGetRelid(rel));
4206+
children=find_inheritance_children(RelationGetRelid(rel),
4207+
AccessExclusiveLock);
42034208

42044209
if (children)
42054210
{
@@ -4213,7 +4218,8 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
42134218
Relationchildrel;
42144219
Form_pg_attributechildatt;
42154220

4216-
childrel=heap_open(childrelid,AccessExclusiveLock);
4221+
/* find_inheritance_children already got lock */
4222+
childrel=heap_open(childrelid,NoLock);
42174223
CheckTableNotInUse(childrel,"ALTER TABLE");
42184224

42194225
tuple=SearchSysCacheCopyAttName(childrelid,colName);
@@ -4509,7 +4515,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
45094515
* routines, we have to do this one level of recursion at a time; we can't
45104516
* use find_all_inheritors to do it in one pass.
45114517
*/
4512-
children=find_inheritance_children(RelationGetRelid(rel));
4518+
children=find_inheritance_children(RelationGetRelid(rel),
4519+
AccessExclusiveLock);
45134520

45144521
/*
45154522
* If we are told not to recurse, there had better not be any child
@@ -4526,7 +4533,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
45264533
Relationchildrel;
45274534
AlteredTableInfo*childtab;
45284535

4529-
childrel=heap_open(childrelid,AccessExclusiveLock);
4536+
/* find_inheritance_children already got lock */
4537+
childrel=heap_open(childrelid,NoLock);
45304538
CheckTableNotInUse(childrel,"ALTER TABLE");
45314539

45324540
/* Find or create work queue entry for this table */
@@ -5426,7 +5434,8 @@ ATExecDropConstraint(Relation rel, const char *constrName,
54265434
* use find_all_inheritors to do it in one pass.
54275435
*/
54285436
if (is_check_constraint)
5429-
children=find_inheritance_children(RelationGetRelid(rel));
5437+
children=find_inheritance_children(RelationGetRelid(rel),
5438+
AccessExclusiveLock);
54305439
else
54315440
children=NIL;
54325441

@@ -5435,7 +5444,8 @@ ATExecDropConstraint(Relation rel, const char *constrName,
54355444
Oidchildrelid=lfirst_oid(child);
54365445
Relationchildrel;
54375446

5438-
childrel=heap_open(childrelid,AccessExclusiveLock);
5447+
/* find_inheritance_children already got lock */
5448+
childrel=heap_open(childrelid,NoLock);
54395449
CheckTableNotInUse(childrel,"ALTER TABLE");
54405450

54415451
ScanKeyInit(&key,
@@ -5659,7 +5669,7 @@ ATPrepAlterColumnType(List **wqueue,
56595669
if (recurse)
56605670
ATSimpleRecursion(wqueue,rel,cmd,recurse);
56615671
elseif (!recursing&&
5662-
find_inheritance_children(RelationGetRelid(rel))!=NIL)
5672+
find_inheritance_children(RelationGetRelid(rel),NoLock)!=NIL)
56635673
ereport(ERROR,
56645674
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
56655675
errmsg("type of inherited column \"%s\" must be changed in child tables too",
@@ -6945,8 +6955,11 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent)
69456955
* exclusive locks on the entire inheritance tree, which is a cure worse
69466956
* than the disease. find_all_inheritors() will cope with circularity
69476957
* anyway, so don't sweat it too much.
6958+
*
6959+
* We use weakest lock we can on child's children, namely AccessShareLock.
69486960
*/
6949-
children=find_all_inheritors(RelationGetRelid(child_rel));
6961+
children=find_all_inheritors(RelationGetRelid(child_rel),
6962+
AccessShareLock);
69506963

69516964
if (list_member_oid(children,RelationGetRelid(parent_rel)))
69526965
ereport(ERROR,

0 commit comments

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