diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c
index c5f035aafe6b4f9a148944777b73c76ef77b983b..49342dc3453ad1570843baa2d1966966f6e1ccf4 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 3b31db76fa216da205caf6dc84758ed6a479e8a7..84b152a20075851e212b61b7608bcc13d7db6d5b 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 76e5cf6596e1e80e5db49030099832851ae32429..c9a5dc2a7442b45ff557e79f561cd9e8c0badc75 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 a067ed36e69bf83470b4a6f07994581133cf516f..09acdaca65ebcb196a434cb503c28337b6dad566 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 81f4aa5aa639712f5df301197fe832baddd5ca41..8513741fa45e0a0fb832e28840813ccd0e1215ce 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 42c3811ef3ee7a05ed2e7c2602bd89cc6449cd09..9773e007922588378359814fcb23887ad2dd4a1c 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 0000000000000000000000000000000000000000..5fb8467a3851a7882078ce005b41ae4f7d77e454
--- /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 */