diff options
| -rw-r--r-- | src/backend/catalog/pg_inherits.c | 45 | ||||
| -rw-r--r-- | src/backend/commands/lockcmds.c | 7 | ||||
| -rw-r--r-- | src/backend/commands/tablecmds.c | 49 | ||||
| -rw-r--r-- | src/backend/optimizer/prep/prepunion.c | 62 | ||||
| -rw-r--r-- | src/backend/parser/parse_coerce.c | 4 | ||||
| -rw-r--r-- | src/include/catalog/pg_inherits.h | 11 | ||||
| -rw-r--r-- | src/include/catalog/pg_inherits_fn.h | 25 |
7 files changed, 135 insertions, 68 deletions
diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index c5f035aafe6..49342dc3453 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/pg_inherits.c,v 1.1 2009/05/12 00:56:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_inherits.c,v 1.2 2009/05/12 03:11:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -22,7 +22,9 @@ #include "access/heapam.h" #include "catalog/pg_class.h" #include "catalog/pg_inherits.h" +#include "catalog/pg_inherits_fn.h" #include "parser/parse_type.h" +#include "storage/lmgr.h" #include "utils/fmgroids.h" #include "utils/syscache.h" #include "utils/tqual.h" @@ -33,9 +35,14 @@ * * Returns a list containing the OIDs of all relations which * inherit *directly* from the relation with OID 'parentrelId'. + * + * The specified lock type is acquired on each child relation (but not on the + * given rel; caller should already have locked it). If lockmode is NoLock + * then no locks are acquired, but caller must beware of race conditions + * against possible DROPs of child relations. */ List * -find_inheritance_children(Oid parentrelId) +find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) { List *list = NIL; Relation relation; @@ -63,13 +70,38 @@ find_inheritance_children(Oid parentrelId) BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(parentrelId)); scan = heap_beginscan(relation, SnapshotNow, 1, key); + while ((inheritsTuple = heap_getnext(scan, ForwardScanDirection)) != NULL) { inhrelid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhrelid; + + if (lockmode != NoLock) + { + /* Get the lock to synchronize against concurrent drop */ + LockRelationOid(inhrelid, lockmode); + + /* + * Now that we have the lock, double-check to see if the relation + * really exists or not. If not, assume it was dropped while + * we waited to acquire lock, and ignore it. + */ + if (!SearchSysCacheExists(RELOID, + ObjectIdGetDatum(inhrelid), + 0, 0, 0)) + { + /* Release useless lock */ + UnlockRelationOid(inhrelid, lockmode); + /* And ignore this relation */ + continue; + } + } + list = lappend_oid(list, inhrelid); } + heap_endscan(scan); heap_close(relation, AccessShareLock); + return list; } @@ -78,9 +110,14 @@ find_inheritance_children(Oid parentrelId) * find_all_inheritors - * Returns a list of relation OIDs including the given rel plus * all relations that inherit from it, directly or indirectly. + * + * The specified lock type is acquired on all child relations (but not on the + * given rel; caller should already have locked it). If lockmode is NoLock + * then no locks are acquired, but caller must beware of race conditions + * against possible DROPs of child relations. */ List * -find_all_inheritors(Oid parentrelId) +find_all_inheritors(Oid parentrelId, LOCKMODE lockmode) { List *rels_list; ListCell *l; @@ -100,7 +137,7 @@ find_all_inheritors(Oid parentrelId) List *currentchildren; /* Get the direct children of this rel */ - currentchildren = find_inheritance_children(currentrel); + currentchildren = find_inheritance_children(currentrel, lockmode); /* * Add to the queue only those children not already seen. This avoids diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index 3b31db76fa2..84b152a2007 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.22 2009/05/12 00:56:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/lockcmds.c,v 1.23 2009/05/12 03:11:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -16,7 +16,7 @@ #include "access/heapam.h" #include "catalog/namespace.h" -#include "catalog/pg_inherits.h" +#include "catalog/pg_inherits_fn.h" #include "commands/lockcmds.h" #include "miscadmin.h" #include "parser/parse_clause.h" @@ -48,8 +48,9 @@ LockTableCommand(LockStmt *lockstmt) reloid = RangeVarGetRelid(relation, false); + /* XXX NoLock here is not really a good idea */ if (recurse) - children_and_self = find_all_inheritors(reloid); + children_and_self = find_all_inheritors(reloid, NoLock); else children_and_self = list_make1_oid(reloid); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 76e5cf6596e..c9a5dc2a744 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.283 2009/05/12 00:56:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.284 2009/05/12 03:11:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -29,6 +29,7 @@ #include "catalog/pg_constraint.h" #include "catalog/pg_depend.h" #include "catalog/pg_inherits.h" +#include "catalog/pg_inherits_fn.h" #include "catalog/pg_namespace.h" #include "catalog/pg_opclass.h" #include "catalog/pg_tablespace.h" @@ -796,7 +797,7 @@ ExecuteTruncate(TruncateStmt *stmt) ListCell *child; List *children; - children = find_all_inheritors(myrelid); + children = find_all_inheritors(myrelid, AccessExclusiveLock); foreach(child, children) { @@ -805,7 +806,8 @@ ExecuteTruncate(TruncateStmt *stmt) if (list_member_oid(relids, childrelid)) continue; - rel = heap_open(childrelid, AccessExclusiveLock); + /* find_all_inheritors already got lock */ + rel = heap_open(childrelid, NoLock); truncate_check_rel(rel); rels = lappend(rels, rel); relids = lappend_oid(relids, childrelid); @@ -1871,7 +1873,7 @@ renameatt(Oid myrelid, ListCell *child; List *children; - children = find_all_inheritors(myrelid); + children = find_all_inheritors(myrelid, AccessExclusiveLock); /* * find_all_inheritors does the recursive search of the inheritance @@ -1895,7 +1897,7 @@ renameatt(Oid myrelid, * tables; else the rename would put them out of step. */ if (!recursing && - find_inheritance_children(myrelid) != NIL) + find_inheritance_children(myrelid, NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited column \"%s\" must be renamed in child tables too", @@ -3289,7 +3291,7 @@ ATSimpleRecursion(List **wqueue, Relation rel, ListCell *child; List *children; - children = find_all_inheritors(relid); + children = find_all_inheritors(relid, AccessExclusiveLock); /* * find_all_inheritors does the recursive search of the inheritance @@ -3303,7 +3305,8 @@ ATSimpleRecursion(List **wqueue, Relation rel, if (childrelid == relid) continue; - childrel = relation_open(childrelid, AccessExclusiveLock); + /* find_all_inheritors already got lock */ + childrel = relation_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); ATPrepCmd(wqueue, childrel, cmd, false, true); relation_close(childrel, NoLock); @@ -3327,14 +3330,15 @@ ATOneLevelRecursion(List **wqueue, Relation rel, ListCell *child; List *children; - children = find_inheritance_children(relid); + children = find_inheritance_children(relid, AccessExclusiveLock); foreach(child, children) { Oid childrelid = lfirst_oid(child); Relation childrel; - childrel = relation_open(childrelid, AccessExclusiveLock); + /* find_inheritance_children already got lock */ + childrel = relation_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); ATPrepCmd(wqueue, childrel, cmd, true, true); relation_close(childrel, NoLock); @@ -3480,7 +3484,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, * If we are told not to recurse, there had better not be any child * tables; else the addition would put them out of step. */ - if (find_inheritance_children(RelationGetRelid(rel)) != NIL) + if (find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("column must be added to child tables too"))); @@ -4199,7 +4203,8 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ - children = find_inheritance_children(RelationGetRelid(rel)); + children = find_inheritance_children(RelationGetRelid(rel), + AccessExclusiveLock); if (children) { @@ -4213,7 +4218,8 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, Relation childrel; Form_pg_attribute childatt; - childrel = heap_open(childrelid, AccessExclusiveLock); + /* find_inheritance_children already got lock */ + childrel = heap_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); tuple = SearchSysCacheCopyAttName(childrelid, colName); @@ -4509,7 +4515,8 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * routines, we have to do this one level of recursion at a time; we can't * use find_all_inheritors to do it in one pass. */ - children = find_inheritance_children(RelationGetRelid(rel)); + children = find_inheritance_children(RelationGetRelid(rel), + AccessExclusiveLock); /* * 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, Relation childrel; AlteredTableInfo *childtab; - childrel = heap_open(childrelid, AccessExclusiveLock); + /* find_inheritance_children already got lock */ + childrel = heap_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); /* Find or create work queue entry for this table */ @@ -5426,7 +5434,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, * use find_all_inheritors to do it in one pass. */ if (is_check_constraint) - children = find_inheritance_children(RelationGetRelid(rel)); + children = find_inheritance_children(RelationGetRelid(rel), + AccessExclusiveLock); else children = NIL; @@ -5435,7 +5444,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, Oid childrelid = lfirst_oid(child); Relation childrel; - childrel = heap_open(childrelid, AccessExclusiveLock); + /* find_inheritance_children already got lock */ + childrel = heap_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); ScanKeyInit(&key, @@ -5659,7 +5669,7 @@ ATPrepAlterColumnType(List **wqueue, if (recurse) ATSimpleRecursion(wqueue, rel, cmd, recurse); else if (!recursing && - find_inheritance_children(RelationGetRelid(rel)) != NIL) + find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("type of inherited column \"%s\" must be changed in child tables too", @@ -6945,8 +6955,11 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent) * exclusive locks on the entire inheritance tree, which is a cure worse * than the disease. find_all_inheritors() will cope with circularity * anyway, so don't sweat it too much. + * + * We use weakest lock we can on child's children, namely AccessShareLock. */ - children = find_all_inheritors(RelationGetRelid(child_rel)); + children = find_all_inheritors(RelationGetRelid(child_rel), + AccessShareLock); if (list_member_oid(children, RelationGetRelid(parent_rel))) ereport(ERROR, diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index a067ed36e69..09acdaca65e 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -22,7 +22,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.169 2009/05/12 00:56:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepunion.c,v 1.170 2009/05/12 03:11:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -32,7 +32,7 @@ #include "access/heapam.h" #include "access/sysattr.h" #include "catalog/namespace.h" -#include "catalog/pg_inherits.h" +#include "catalog/pg_inherits_fn.h" #include "catalog/pg_type.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -1157,8 +1157,29 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) return; } - /* Scan for all members of inheritance set */ - inhOIDs = find_all_inheritors(parentOID); + /* + * The rewriter should already have obtained an appropriate lock on each + * relation named in the query. However, for each child relation we add + * to the query, we must obtain an appropriate lock, because this will be + * the first use of those relations in the parse/rewrite/plan pipeline. + * + * If the parent relation is the query's result relation, then we need + * RowExclusiveLock. Otherwise, if it's accessed FOR UPDATE/SHARE, we + * need RowShareLock; otherwise AccessShareLock. We can't just grab + * AccessShareLock because then the executor would be trying to upgrade + * the lock, leading to possible deadlocks. (This code should match the + * parser and rewriter.) + */ + oldrc = get_rowmark(parse, rti); + if (rti == parse->resultRelation) + lockmode = RowExclusiveLock; + else if (oldrc) + lockmode = RowShareLock; + else + lockmode = AccessShareLock; + + /* Scan for all members of inheritance set, acquire needed locks */ + inhOIDs = find_all_inheritors(parentOID, lockmode); /* * Check that there's at least one descendant, else treat as no-child @@ -1173,40 +1194,19 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) } /* - * Find out if parent relation is selected FOR UPDATE/SHARE. If so, - * we need to mark its RowMarkClause as isParent = true, and generate - * a new RowMarkClause for each child. + * If parent relation is selected FOR UPDATE/SHARE, we need to mark its + * RowMarkClause as isParent = true, and generate a new RowMarkClause for + * each child. */ - oldrc = get_rowmark(parse, rti); if (oldrc) oldrc->isParent = true; /* * Must open the parent relation to examine its tupdesc. We need not lock - * it since the rewriter already obtained at least AccessShareLock on each - * relation used in the query. + * it; we assume the rewriter already did. */ oldrelation = heap_open(parentOID, NoLock); - /* - * However, for each child relation we add to the query, we must obtain an - * appropriate lock, because this will be the first use of those relations - * in the parse/rewrite/plan pipeline. - * - * If the parent relation is the query's result relation, then we need - * RowExclusiveLock. Otherwise, if it's accessed FOR UPDATE/SHARE, we - * need RowShareLock; otherwise AccessShareLock. We can't just grab - * AccessShareLock because then the executor would be trying to upgrade - * the lock, leading to possible deadlocks. (This code should match the - * parser and rewriter.) - */ - if (rti == parse->resultRelation) - lockmode = RowExclusiveLock; - else if (oldrc) - lockmode = RowShareLock; - else - lockmode = AccessShareLock; - /* Scan the inheritance set and expand it */ appinfos = NIL; foreach(l, inhOIDs) @@ -1217,9 +1217,9 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) Index childRTindex; AppendRelInfo *appinfo; - /* Open rel, acquire the appropriate lock type */ + /* Open rel if needed; we already have required locks */ if (childOID != parentOID) - newrelation = heap_open(childOID, lockmode); + newrelation = heap_open(childOID, NoLock); else newrelation = oldrelation; diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index 81f4aa5aa63..8513741fa45 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -8,14 +8,14 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.175 2009/05/12 00:56:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.176 2009/05/12 03:11:02 tgl Exp $ * *------------------------------------------------------------------------- */ #include "postgres.h" #include "catalog/pg_cast.h" -#include "catalog/pg_inherits.h" +#include "catalog/pg_inherits_fn.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "nodes/makefuncs.h" diff --git a/src/include/catalog/pg_inherits.h b/src/include/catalog/pg_inherits.h index 42c3811ef3e..9773e007922 100644 --- a/src/include/catalog/pg_inherits.h +++ b/src/include/catalog/pg_inherits.h @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/pg_inherits.h,v 1.27 2009/05/12 00:56:05 tgl Exp $ + * $PostgreSQL: pgsql/src/include/catalog/pg_inherits.h,v 1.28 2009/05/12 03:11:02 tgl Exp $ * * NOTES * the genbki.sh script reads this file and generates .bki @@ -20,7 +20,6 @@ #define PG_INHERITS_H #include "catalog/genbki.h" -#include "nodes/pg_list.h" /* ---------------- * pg_inherits definition. cpp turns this into @@ -57,12 +56,4 @@ typedef FormData_pg_inherits *Form_pg_inherits; * ---------------- */ -/* - * prototypes for functions in pg_inherits.c - */ -extern List *find_inheritance_children(Oid parentrelId); -extern List *find_all_inheritors(Oid parentrelId); -extern bool has_subclass(Oid relationId); -extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId); - #endif /* PG_INHERITS_H */ diff --git a/src/include/catalog/pg_inherits_fn.h b/src/include/catalog/pg_inherits_fn.h new file mode 100644 index 00000000000..5fb8467a385 --- /dev/null +++ b/src/include/catalog/pg_inherits_fn.h @@ -0,0 +1,25 @@ +/*------------------------------------------------------------------------- + * + * pg_inherits_fn.h + * prototypes for functions in catalog/pg_inherits.c + * + * + * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * $PostgreSQL: pgsql/src/include/catalog/pg_inherits_fn.h,v 1.1 2009/05/12 03:11:02 tgl Exp $ + * + *------------------------------------------------------------------------- + */ +#ifndef PG_INHERITS_FN_H +#define PG_INHERITS_FN_H + +#include "nodes/pg_list.h" +#include "storage/lock.h" + +extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode); +extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode); +extern bool has_subclass(Oid relationId); +extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId); + +#endif /* PG_INHERITS_FN_H */ |