From 2e8936a89e9bbc8bd25e611617b435212402bc2b Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Thu, 5 Apr 2018 16:45:49 +0000 Subject: [PATCH v7 01/12] Add skip_locked argument to find_inheritance_children(). --- src/backend/catalog/partition.c | 2 +- src/backend/catalog/pg_inherits.c | 26 +++++++++++++++++++++++--- src/backend/commands/lockcmds.c | 2 +- src/backend/commands/tablecmds.c | 16 ++++++++-------- src/backend/commands/trigger.c | 2 +- src/include/catalog/pg_inherits_fn.h | 3 ++- 6 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 39ee773d93..42453c5007 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -232,7 +232,7 @@ RelationBuildPartitionDesc(Relation rel) PartitionRangeBound **rbounds = NULL; /* Get partition oids from pg_inherits */ - inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock); + inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock, false, NULL); /* Collect bound spec nodes in a list */ i = 0; diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index 5a5beb9273..17f8b58fba 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -52,9 +52,14 @@ typedef struct SeenRelsEntry * 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. + * + * If skip_locked is true, any child relations that cannot be locked immediately + * without waiting are not added to the returned OID list, and skipped will be + * set to true if it is provided. Otherwise, skipped will be set to false if it + * is provided. */ List * -find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) +find_inheritance_children(Oid parentrelId, LOCKMODE lockmode, bool skip_locked, bool *skipped) { List *list = NIL; Relation relation; @@ -66,13 +71,19 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) int maxoids, numoids, i; + bool did_skip = false; /* * Can skip the scan if pg_class shows the relation has never had a * subclass. */ if (!has_subclass(parentrelId)) + { + if (skipped != NULL) + *skipped = did_skip; + return NIL; + } /* * Scan pg_inherits and build a working array of subclass OIDs. @@ -125,7 +136,13 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) if (lockmode != NoLock) { /* Get the lock to synchronize against concurrent drop */ - LockRelationOid(inhrelid, lockmode); + if (!skip_locked) + LockRelationOid(inhrelid, lockmode); + else if (!ConditionalLockRelationOid(inhrelid, lockmode)) + { + did_skip = true; + continue; + } /* * Now that we have the lock, double-check to see if the relation @@ -146,6 +163,9 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) pfree(oidarr); + if (skipped != NULL) + *skipped = did_skip; + return list; } @@ -200,7 +220,7 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) ListCell *lc; /* Get the direct children of this rel */ - currentchildren = find_inheritance_children(currentrel, lockmode); + currentchildren = find_inheritance_children(currentrel, lockmode, false, NULL); /* * 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 b247c0fe2e..1039159946 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -120,7 +120,7 @@ LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, Oid userid) List *children; ListCell *lc; - children = find_inheritance_children(reloid, NoLock); + children = find_inheritance_children(reloid, NoLock, false, NULL); foreach(lc, children) { diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index ec2f9616ed..15017c5a62 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -2694,7 +2694,7 @@ renameatt_internal(Oid myrelid, * expected_parents will only be 0 if we are not already recursing. */ if (expected_parents == 0 && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, NoLock, false, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited column \"%s\" must be renamed in child tables too", @@ -2893,7 +2893,7 @@ rename_constraint_internal(Oid myrelid, else { if (expected_parents == 0 && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, NoLock, false, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited constraint \"%s\" must be renamed in child tables too", @@ -5354,7 +5354,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, */ if (colDef->identity && recurse && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, NoLock, false, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot recursively add identity column to table that has child tables"))); @@ -5597,7 +5597,7 @@ ATExecAddColumn(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), lockmode); + children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL); /* * If we are told not to recurse, there had better not be any child @@ -6693,7 +6693,7 @@ 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), lockmode); + children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL); if (children) { @@ -7144,7 +7144,7 @@ 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), lockmode); + children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL); /* * Check if ONLY was specified with ALTER TABLE. If so, allow the @@ -8831,7 +8831,7 @@ ATExecDropConstraint(Relation rel, const char *constrName, * use find_all_inheritors to do it in one pass. */ if (!is_no_inherit_constraint) - children = find_inheritance_children(RelationGetRelid(rel), lockmode); + children = find_inheritance_children(RelationGetRelid(rel), lockmode, false, NULL); else children = NIL; @@ -9173,7 +9173,7 @@ ATPrepAlterColumnType(List **wqueue, } } else if (!recursing && - find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) + find_inheritance_children(RelationGetRelid(rel), NoLock, false, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("type of inherited column \"%s\" must be changed in child tables too", diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index a189356cad..a7e549e0ae 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1115,7 +1115,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, ListCell *l; List *idxs = NIL; - idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock); + idxs = find_inheritance_children(indexOid, ShareRowExclusiveLock, false, NULL); foreach(l, idxs) childTbls = lappend_oid(childTbls, IndexGetRelation(lfirst_oid(l), diff --git a/src/include/catalog/pg_inherits_fn.h b/src/include/catalog/pg_inherits_fn.h index eebee977a5..34d2a2f39f 100644 --- a/src/include/catalog/pg_inherits_fn.h +++ b/src/include/catalog/pg_inherits_fn.h @@ -17,7 +17,8 @@ #include "nodes/pg_list.h" #include "storage/lock.h" -extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode); +extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode, + bool skip_locked, bool *skipped); extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **parents); extern bool has_subclass(Oid relationId); -- 2.16.2