From b7ec1ddc2e26e75e0ab092c36461c09e9ca0a9d8 Mon Sep 17 00:00:00 2001 From: amit Date: Tue, 8 Aug 2017 18:42:30 +0900 Subject: [PATCH 2/5] Teach pg_inherits.c a bit about partitioning Both find_inheritance_children and find_all_inheritors now list partitioned child tables before non-partitioned ones and return the number of partitioned tables in an optional output argument --- contrib/sepgsql/dml.c | 2 +- src/backend/catalog/partition.c | 2 +- src/backend/catalog/pg_inherits.c | 157 ++++++++++++++++++++++++++------- src/backend/commands/analyze.c | 3 +- src/backend/commands/lockcmds.c | 2 +- src/backend/commands/publicationcmds.c | 2 +- src/backend/commands/tablecmds.c | 39 ++++---- src/backend/commands/vacuum.c | 3 +- src/backend/optimizer/prep/prepunion.c | 2 +- src/include/catalog/pg_inherits_fn.h | 5 +- 10 files changed, 162 insertions(+), 55 deletions(-) diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c index b643720e36..6fc279805c 100644 --- a/contrib/sepgsql/dml.c +++ b/contrib/sepgsql/dml.c @@ -333,7 +333,7 @@ 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); + tableIds = find_all_inheritors(rte->relid, NoLock, NULL, NULL); foreach(li, tableIds) { diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 3d72d08c35..465e4fc097 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -196,7 +196,7 @@ RelationBuildPartitionDesc(Relation rel) return; /* Get partition oids from pg_inherits */ - inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock); + inhoids = find_inheritance_children(RelationGetRelid(rel), NoLock, 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 245a374fc9..99b1e70de6 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -30,9 +30,12 @@ #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/memutils.h" +#include "utils/lsyscache.h" #include "utils/syscache.h" #include "utils/tqual.h" +static int32 inhchildinfo_cmp(const void *p1, const void *p2); + /* * Entry of a hash table used in find_all_inheritors. See below. */ @@ -42,6 +45,30 @@ typedef struct SeenRelsEntry ListCell *numparents_cell; /* corresponding list cell */ } SeenRelsEntry; +/* Information about one inheritance child table. */ +typedef struct InhChildInfo +{ + Oid relid; + bool is_partitioned; +} InhChildInfo; + +#define OID_CMP(o1, o2) \ + ((o1) < (o2) ? -1 : ((o1) > (o2) ? 1 : 0)); + +static int32 +inhchildinfo_cmp(const void *p1, const void *p2) +{ + InhChildInfo c1 = *((const InhChildInfo *) p1); + InhChildInfo c2 = *((const InhChildInfo *) p2); + + if (c1.is_partitioned && !c2.is_partitioned) + return -1; + if (!c1.is_partitioned && c2.is_partitioned) + return 1; + + return OID_CMP(c1.relid, c2.relid); +} + /* * find_inheritance_children * @@ -54,7 +81,8 @@ typedef struct SeenRelsEntry * against possible DROPs of child relations. */ List * -find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) +find_inheritance_children(Oid parentrelId, LOCKMODE lockmode, + int *num_partitioned_children) { List *list = NIL; Relation relation; @@ -62,9 +90,10 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) ScanKeyData key[1]; HeapTuple inheritsTuple; Oid inhrelid; - Oid *oidarr; - int maxoids, - numoids, + InhChildInfo *inhchildren; + int maxchildren, + numchildren, + my_num_partitioned_children, i; /* @@ -77,9 +106,10 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) /* * Scan pg_inherits and build a working array of subclass OIDs. */ - maxoids = 32; - oidarr = (Oid *) palloc(maxoids * sizeof(Oid)); - numoids = 0; + maxchildren = 32; + inhchildren = (InhChildInfo *) palloc(maxchildren * sizeof(InhChildInfo)); + numchildren = 0; + my_num_partitioned_children = 0; relation = heap_open(InheritsRelationId, AccessShareLock); @@ -94,33 +124,47 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) while ((inheritsTuple = systable_getnext(scan)) != NULL) { inhrelid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhrelid; - if (numoids >= maxoids) + if (numchildren >= maxchildren) + { + maxchildren *= 2; + inhchildren = (InhChildInfo *) repalloc(inhchildren, + maxchildren * sizeof(InhChildInfo)); + } + inhchildren[numchildren].relid = inhrelid; + + if (get_rel_relkind(inhrelid) == RELKIND_PARTITIONED_TABLE) { - maxoids *= 2; - oidarr = (Oid *) repalloc(oidarr, maxoids * sizeof(Oid)); + inhchildren[numchildren].is_partitioned = true; + my_num_partitioned_children++; } - oidarr[numoids++] = inhrelid; + else + inhchildren[numchildren].is_partitioned = false; + numchildren++; } systable_endscan(scan); heap_close(relation, AccessShareLock); + if (num_partitioned_children) + *num_partitioned_children = my_num_partitioned_children; + /* * If we found more than one child, sort them by OID. This ensures * reasonably consistent behavior regardless of the vagaries of an * indexscan. This is important since we need to be sure all backends * lock children in the same order to avoid needless deadlocks. */ - if (numoids > 1) - qsort(oidarr, numoids, sizeof(Oid), oid_cmp); + if (numchildren > 1) + qsort(inhchildren, numchildren, sizeof(InhChildInfo), + inhchildinfo_cmp); /* * Acquire locks and build the result list. */ - for (i = 0; i < numoids; i++) + for (i = 0; i < numchildren; i++) { - inhrelid = oidarr[i]; + inhrelid = inhchildren[i].relid; if (lockmode != NoLock) { @@ -144,7 +188,7 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) list = lappend_oid(list, inhrelid); } - pfree(oidarr); + pfree(inhchildren); return list; } @@ -159,18 +203,28 @@ find_inheritance_children(Oid parentrelId, LOCKMODE lockmode) * given rel. * * 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. + * given rel; caller should already have locked it), unless + * lock_only_partitioned_children is specified, in which case, only the + * child relations that are partitioned tables are locked. If lockmode is + * NoLock then no locks are acquired, but caller must beware of race + * conditions against possible DROPs of child relations. + * + * Returned list of OIDs is such that all the partitioned tables in the tree + * appear at the head of the list. If num_partitioned_children is non-NULL, + * *num_partitioned_children returns the number of partitioned child table + * OIDs at the head of the list. */ List * -find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) +find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, + List **numparents, int *num_partitioned_children) { /* hash table for O(1) rel_oid -> rel_numparents cell lookup */ HTAB *seen_rels; HASHCTL ctl; List *rels_list, - *rel_numparents; + *rel_numparents, + *partitioned_rels_list, + *other_rels_list; ListCell *l; memset(&ctl, 0, sizeof(ctl)); @@ -185,31 +239,71 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) /* * We build a list starting with the given rel and adding all direct and - * indirect children. We can use a single list as both the record of - * already-found rels and the agenda of rels yet to be scanned for more - * children. This is a bit tricky but works because the foreach() macro - * doesn't fetch the next list element until the bottom of the loop. + * indirect children. We can use a single list (rels_list) as both the + * record of already-found rels and the agenda of rels yet to be scanned + * for more children. This is a bit tricky but works because the foreach() + * macro doesn't fetch the next list element until the bottom of the loop. + * + * partitioned_child_rels will contain the OIDs of the partitioned child + * tables and other_rels_list will contain the OIDs of the non-partitioned + * child tables. Result list will be generated by concatening the two + * lists together with partitioned_child_rels appearing first. */ rels_list = list_make1_oid(parentrelId); + partitioned_rels_list = list_make1_oid(parentrelId); + other_rels_list = NIL; rel_numparents = list_make1_int(0); + if (num_partitioned_children) + *num_partitioned_children = 0; + foreach(l, rels_list) { Oid currentrel = lfirst_oid(l); List *currentchildren; - ListCell *lc; + ListCell *lc, + *first_nonpartitioned_child; + int cur_num_partitioned_children = 0, + i; /* Get the direct children of this rel */ - currentchildren = find_inheritance_children(currentrel, lockmode); + currentchildren = find_inheritance_children(currentrel, lockmode, + &cur_num_partitioned_children); + + if (num_partitioned_children) + *num_partitioned_children += cur_num_partitioned_children; + + /* + * Append partitioned children to rels_list and partitioned_rels_list. + * We know for sure that partitioned children don't need the + * the de-duplication logic in the following loop, because partitioned + * tables are not allowed to partiticipate in multiple inheritance. + */ + i = 0; + foreach(lc, currentchildren) + { + if (i < cur_num_partitioned_children) + { + Oid child_oid = lfirst_oid(lc); + + rels_list = lappend_oid(rels_list, child_oid); + partitioned_rels_list = lappend_oid(partitioned_rels_list, + child_oid); + } + else + break; + i++; + } + first_nonpartitioned_child = lc; /* * Add to the queue only those children not already seen. This avoids * making duplicate entries in case of multiple inheritance paths from * the same parent. (It'll also keep us from getting into an infinite * loop, though theoretically there can't be any cycles in the - * inheritance graph anyway.) + * inheritance graph anyway.) Also, add them to the other_rels_list. */ - foreach(lc, currentchildren) + for_each_cell(lc, first_nonpartitioned_child) { Oid child_oid = lfirst_oid(lc); bool found; @@ -225,6 +319,7 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) { /* if it's not there, add it. expect 1 parent, initially. */ rels_list = lappend_oid(rels_list, child_oid); + other_rels_list = lappend_oid(other_rels_list, child_oid); rel_numparents = lappend_int(rel_numparents, 1); hash_entry->numparents_cell = rel_numparents->tail; } @@ -237,8 +332,10 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) list_free(rel_numparents); hash_destroy(seen_rels); + list_free(rels_list); - return rels_list; + /* List partitioned child tables before non-partitioned ones. */ + return list_concat(partitioned_rels_list, other_rels_list); } diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 2b638271b3..ae8ce71e1c 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -1282,7 +1282,8 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, * the children. */ tableOIDs = - find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, NULL); + find_all_inheritors(RelationGetRelid(onerel), AccessShareLock, 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 9fe9e022b0..529f244f7e 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); + children = find_inheritance_children(reloid, NoLock, NULL); foreach(lc, children) { diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 610cb499d2..64179ea3ef 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, NULL); foreach(child, children) { diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1b8d4b3d17..14bac087d9 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1231,7 +1231,8 @@ ExecuteTruncate(TruncateStmt *stmt) ListCell *child; List *children; - children = find_all_inheritors(myrelid, AccessExclusiveLock, NULL); + children = find_all_inheritors(myrelid, AccessExclusiveLock, NULL, + NULL); foreach(child, children) { @@ -2556,7 +2557,7 @@ renameatt_internal(Oid myrelid, * outside the inheritance hierarchy being processed. */ child_oids = find_all_inheritors(myrelid, AccessExclusiveLock, - &child_numparents); + &child_numparents, NULL); /* * find_all_inheritors does the recursive search of the inheritance @@ -2583,7 +2584,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, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited column \"%s\" must be renamed in child tables too", @@ -2766,7 +2767,7 @@ rename_constraint_internal(Oid myrelid, *li; child_oids = find_all_inheritors(myrelid, AccessExclusiveLock, - &child_numparents); + &child_numparents, NULL); forboth(lo, child_oids, li, child_numparents) { @@ -2782,7 +2783,7 @@ rename_constraint_internal(Oid myrelid, else { if (expected_parents == 0 && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, NoLock, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited constraint \"%s\" must be renamed in child tables too", @@ -4790,7 +4791,7 @@ ATSimpleRecursion(List **wqueue, Relation rel, ListCell *child; List *children; - children = find_all_inheritors(relid, lockmode, NULL); + children = find_all_inheritors(relid, lockmode, NULL, NULL); /* * find_all_inheritors does the recursive search of the inheritance @@ -5186,7 +5187,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, */ if (colDef->identity && recurse && - find_inheritance_children(myrelid, NoLock) != NIL) + find_inheritance_children(myrelid, NoLock, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot recursively add identity column to table that has child tables"))); @@ -5392,7 +5393,8 @@ 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, + NULL); /* * If we are told not to recurse, there had better not be any child @@ -6511,7 +6513,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), lockmode); + children = find_inheritance_children(RelationGetRelid(rel), lockmode, + NULL); if (children) { @@ -6945,7 +6948,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), lockmode); + children = find_inheritance_children(RelationGetRelid(rel), lockmode, + NULL); /* * Check if ONLY was specified with ALTER TABLE. If so, allow the @@ -7664,7 +7668,7 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, */ if (!recursing && !con->connoinherit) children = find_all_inheritors(RelationGetRelid(rel), - lockmode, NULL); + lockmode, NULL, NULL); /* * For CHECK constraints, we must ensure that we only mark the @@ -8547,7 +8551,8 @@ 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, + NULL); else children = NIL; @@ -8836,7 +8841,7 @@ ATPrepAlterColumnType(List **wqueue, ListCell *child; List *children; - children = find_all_inheritors(relid, lockmode, NULL); + children = find_all_inheritors(relid, lockmode, NULL, NULL); /* * find_all_inheritors does the recursive search of the inheritance @@ -8887,7 +8892,8 @@ ATPrepAlterColumnType(List **wqueue, } } else if (!recursing && - find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL) + find_inheritance_children(RelationGetRelid(rel), + NoLock, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("type of inherited column \"%s\" must be changed in child tables too", @@ -10997,7 +11003,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); + AccessShareLock, NULL, NULL); if (list_member_oid(children, RelationGetRelid(parent_rel))) ereport(ERROR, @@ -13503,7 +13509,8 @@ 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, NULL, + NULL); if (list_member_oid(attachrel_children, RelationGetRelid(rel))) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_TABLE), diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index faa181207a..e2e5ffce42 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -430,7 +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)); + find_all_inheritors(relid, NoLock, NULL, + NULL)); else oid_list = lappend_oid(oid_list, relid); MemoryContextSwitchTo(oldcontext); diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index cf46b74782..09e45c2982 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -1418,7 +1418,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); + inhOIDs = find_all_inheritors(parentOID, lockmode, 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 7743388899..8f371acae7 100644 --- a/src/include/catalog/pg_inherits_fn.h +++ b/src/include/catalog/pg_inherits_fn.h @@ -17,9 +17,10 @@ #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, + int *num_partitioned_children); extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, - List **parents); + List **parents, int *num_partitioned_children); extern bool has_subclass(Oid relationId); extern bool has_superclass(Oid relationId); extern bool typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId); -- 2.11.0