From b92ef9ec07841942ceedd173dd81e3286419202f Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 10 Aug 2017 17:59:18 +0900 Subject: [PATCH 2/6] Allow locking only partitioned children in partition tree find_inheritance_childrem will still return the OIDs of the non-partitioned children, but does not lock them if the caller asks it so. None of the callers pass 'true' yet though. --- contrib/sepgsql/dml.c | 3 ++- src/backend/catalog/partition.c | 3 ++- src/backend/catalog/pg_inherits.c | 20 ++++++++++++++++---- src/backend/commands/analyze.c | 4 ++-- src/backend/commands/lockcmds.c | 2 +- src/backend/commands/publicationcmds.c | 2 +- src/backend/commands/tablecmds.c | 34 +++++++++++++++++----------------- src/backend/commands/vacuum.c | 4 ++-- src/backend/executor/execMain.c | 4 ++-- src/backend/optimizer/prep/prepunion.c | 2 +- src/include/catalog/pg_inherits_fn.h | 2 ++ 11 files changed, 48 insertions(+), 32 deletions(-) diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c index 6fc279805c..91f338f8bf 100644 --- a/contrib/sepgsql/dml.c +++ b/contrib/sepgsql/dml.c @@ -333,7 +333,8 @@ sepgsql_dml_privileges(List *rangeTabls, bool abort_on_violation) if (!rte->inh) tableIds = list_make1_oid(rte->relid); else - tableIds = find_all_inheritors(rte->relid, NoLock, NULL, NULL); + tableIds = find_all_inheritors(rte->relid, NoLock, false, + NULL, NULL); foreach(li, tableIds) { diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index fe8e60de14..9645381fcb 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -178,7 +178,8 @@ RelationBuildPartitionDesc(Relation rel) return; /* Get partition oids from pg_inherits */ - inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock, NULL); + 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 5292ec8058..72420f65f1 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -74,13 +74,16 @@ inhchildinfo_cmp(const void *p1, const void *p2) * 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. + * The specified lock type is acquired on each child relation, (but not on the + * given rel; caller should already have locked it), unless + * lock_only_partitioned_children is specified in which case only partitioned + * children are locked. 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, LOCKMODE lockmode, + bool lock_only_partitioned_children, int *num_partitioned_children) { List *list = NIL; @@ -162,6 +165,13 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode, { inhrelid = inhchildren[i].relid; + /* If requested, skip locking non-partitioned children. */ + if (lock_only_partitioned_children && i >= *num_partitioned_children) + { + list = lappend_oid(list, inhrelid); + continue; + } + if (lockmode != NoLock) { /* Get the lock to synchronize against concurrent drop */ @@ -212,6 +222,7 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode, */ List * find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, + bool lock_only_partitioned_children, List **numparents, int *num_partitioned_children) { /* hash table for O(1) rel_oid -> rel_numparents cell lookup */ @@ -264,6 +275,7 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, /* Get the direct children of this rel */ currentchildren = find_inheritance_children(currentrel, lockmode, + lock_only_partitioned_children, &cur_num_partitioned_children); my_num_partitioned_children += cur_num_partitioned_children; diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 10cc2b8314..4bd374632f 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -1282,8 +1282,8 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, * the children. */ tableOIDs = - find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, NULL, - NULL); + find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, false, + NULL, NULL); /* * Check that there's at least one descendant, else fail. This could diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index 529f244f7e..771aa11b1c 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -112,7 +112,7 @@ LockTableRecurse(Oid reloid, LOCKMODE lockmode, bool nowait) List *children; ListCell *lc; - children = find_inheritance_children(reloid, NoLock, NULL); + children = find_inheritance_children(reloid, NoLock, false, NULL); foreach(lc, children) { diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 64179ea3ef..4315028c66 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -516,7 +516,7 @@ OpenTableList(List *tables) List *children; children = find_all_inheritors(myrelid, ShareUpdateExclusiveLock, - NULL, NULL); + false, NULL, NULL); foreach(child, children) { diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 4d686a6f71..ef3869854a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1239,8 +1239,8 @@ ExecuteTruncate(TruncateStmt *stmt) ListCell *child; List *children; - children = find_all_inheritors(myrelid, AccessExclusiveLock, NULL, - NULL); + children = find_all_inheritors(myrelid, AccessExclusiveLock, false, + NULL, NULL); foreach(child, children) { @@ -2567,7 +2567,7 @@ renameatt_internal(Oid myrelid, * calls to renameatt() can determine whether there are any parents * outside the inheritance hierarchy being processed. */ - child_oids = find_all_inheritors(myrelid, AccessExclusiveLock, + child_oids = find_all_inheritors(myrelid, AccessExclusiveLock, false, &child_numparents, NULL); /* @@ -2595,7 +2595,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, NULL) != 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", @@ -2778,7 +2778,7 @@ rename_constraint_internal(Oid myrelid, *li; child_oids = find_all_inheritors(myrelid, AccessExclusiveLock, - &child_numparents, NULL); + false, &child_numparents, NULL); forboth(lo, child_oids, li, child_numparents) { @@ -2794,7 +2794,7 @@ rename_constraint_internal(Oid myrelid, else { if (expected_parents == 0 && - find_inheritance_children(myrelid, NoLock, NULL) != 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", @@ -4807,7 +4807,7 @@ ATSimpleRecursion(List **wqueue, Relation rel, ListCell *child; List *children; - children = find_all_inheritors(relid, lockmode, NULL, NULL); + children = find_all_inheritors(relid, lockmode, false, NULL, NULL); /* * find_all_inheritors does the recursive search of the inheritance @@ -5216,7 +5216,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, */ if (colDef->identity && recurse && - find_inheritance_children(myrelid, NoLock, NULL) != 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"))); @@ -5423,7 +5423,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, * use find_all_inheritors to do it in one pass. */ children = find_inheritance_children(RelationGetRelid(rel), lockmode, - NULL); + false, NULL); /* * If we are told not to recurse, there had better not be any child @@ -6543,7 +6543,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, * use find_all_inheritors to do it in one pass. */ children = find_inheritance_children(RelationGetRelid(rel), lockmode, - NULL); + false, NULL); if (children) { @@ -6978,7 +6978,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * use find_all_inheritors to do it in one pass. */ children = find_inheritance_children(RelationGetRelid(rel), lockmode, - NULL); + false, NULL); /* * Check if ONLY was specified with ALTER TABLE. If so, allow the @@ -7699,7 +7699,7 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, */ if (!recursing && !con->connoinherit) children = find_all_inheritors(RelationGetRelid(rel), - lockmode, NULL, NULL); + lockmode, false, NULL, NULL); /* * For CHECK constraints, we must ensure that we only mark the @@ -8583,7 +8583,7 @@ ATExecDropConstraint(Relation rel, const char *constrName, */ if (!is_no_inherit_constraint) children = find_inheritance_children(RelationGetRelid(rel), lockmode, - NULL); + false, NULL); else children = NIL; @@ -8872,7 +8872,7 @@ ATPrepAlterColumnType(List **wqueue, ListCell *child; List *children; - children = find_all_inheritors(relid, lockmode, NULL, NULL); + children = find_all_inheritors(relid, lockmode, false, NULL, NULL); /* * find_all_inheritors does the recursive search of the inheritance @@ -8924,7 +8924,7 @@ ATPrepAlterColumnType(List **wqueue, } else if (!recursing && find_inheritance_children(RelationGetRelid(rel), - NoLock, NULL) != NIL) + NoLock, false, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("type of inherited column \"%s\" must be changed in child tables too", @@ -11036,7 +11036,7 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode) * We use weakest lock we can on child's children, namely AccessShareLock. */ children = find_all_inheritors(RelationGetRelid(child_rel), - AccessShareLock, NULL, NULL); + AccessShareLock, false, NULL, NULL); if (list_member_oid(children, RelationGetRelid(parent_rel))) ereport(ERROR, @@ -13707,7 +13707,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) * weaker lock now and the stronger one only when needed. */ attachrel_children = find_all_inheritors(RelationGetRelid(attachrel), - AccessExclusiveLock, NULL, + AccessExclusiveLock, false, NULL, NULL); if (list_member_oid(attachrel_children, RelationGetRelid(rel))) ereport(ERROR, diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index e2e5ffce42..70cd5721f3 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -430,8 +430,8 @@ get_rel_oids(Oid relid, const RangeVar *vacrel) oldcontext = MemoryContextSwitchTo(vac_context); if (include_parts) oid_list = list_concat(oid_list, - find_all_inheritors(relid, NoLock, NULL, - NULL)); + find_all_inheritors(relid, NoLock, false, + NULL, NULL)); else oid_list = lappend_oid(oid_list, relid); MemoryContextSwitchTo(oldcontext); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b63abba1e4..9ee0e03a3c 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -3278,8 +3278,8 @@ ExecSetupPartitionTupleRouting(Relation rel, * Get the information about the partition tree after locking all the * partitions. */ - (void) find_all_inheritors(RelationGetRelid(rel), RowExclusiveLock, NULL, - NULL); + (void) find_all_inheritors(RelationGetRelid(rel), RowExclusiveLock, false, + NULL, NULL); pds = RelationGetPartitionDispatchInfo(rel, num_parted, &leaf_parts); /* diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 81865cad7d..0d20ffa2f7 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1425,7 +1425,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti) lockmode = AccessShareLock; /* Scan for all members of inheritance set, acquire needed locks */ - inhOIDs = find_all_inheritors(parentOID, lockmode, NULL, NULL); + inhOIDs = find_all_inheritors(parentOID, lockmode, false, NULL, NULL); /* * Check that there's at least one descendant, else treat as no-child diff --git a/src/include/catalog/pg_inherits_fn.h b/src/include/catalog/pg_inherits_fn.h index 8f371acae7..e568d11e43 100644 --- a/src/include/catalog/pg_inherits_fn.h +++ b/src/include/catalog/pg_inherits_fn.h @@ -18,8 +18,10 @@ #include "storage/lock.h" extern List *find_inheritance_children(Oid parentrelId, LOCKMODE lockmode, + bool lock_only_partitioned_children, int *num_partitioned_children); extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, + bool lock_only_partitioned_children, List **parents, int *num_partitioned_children); extern bool has_subclass(Oid relationId); extern bool has_superclass(Oid relationId); -- 2.11.0