From ecec91fc4744ddd9a231754afdf10ec09742f8d9 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 19 Mar 2021 10:52:26 -0300 Subject: [PATCH] Fix for generation of invalid constraints per report from Justin Pryzby --- src/backend/catalog/heap.c | 2 +- src/backend/catalog/partition.c | 32 ++++++++++++++--------------- src/backend/commands/tablecmds.c | 12 +++++------ src/backend/utils/cache/partcache.c | 11 ++-------- src/include/catalog/partition.h | 2 +- 5 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 1a05f2a2fe..c6c5e40a39 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1919,7 +1919,7 @@ heap_drop_with_catalog(Oid relid) elog(ERROR, "cache lookup failed for relation %u", relid); if (((Form_pg_class) GETSTRUCT(tuple))->relispartition) { - parentOid = get_partition_parent(relid); + parentOid = get_partition_parent(relid, true); LockRelationOid(parentOid, AccessExclusiveLock); /* diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 797a7384ad..a7cf3d9e86 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -42,15 +42,15 @@ static void get_partition_ancestors_worker(Relation inhRel, Oid relid, * * Returns inheritance parent of a partition by scanning pg_inherits * - * If the partition is in the process of being detached, InvalidOid is - * returned and no error is thrown. + * If the partition is in the process of being detached, we raise the same + * error unless even_if_detached is passed. * * Note: Because this function assumes that the relation whose OID is passed * as an argument will have precisely one parent, it should only be called * when it is known that the relation is a partition. */ Oid -get_partition_parent(Oid relid) +get_partition_parent(Oid relid, bool even_if_detached) { Relation catalogRelation; Oid result; @@ -60,10 +60,10 @@ get_partition_parent(Oid relid) result = get_partition_parent_worker(catalogRelation, relid, &detached); - if (detached) - result = InvalidOid; - else if (!OidIsValid(result)) + if (!OidIsValid(result)) elog(ERROR, "could not find tuple for parent of relation %u", relid); + if (detached && !even_if_detached) + elog(ERROR, "relation %u has no parent because it's being detached", relid); table_close(catalogRelation, AccessShareLock); @@ -75,8 +75,8 @@ get_partition_parent(Oid relid) * Scan the pg_inherits relation to return the OID of the parent of the * given relation * - * If the partition is being detached, InvalidOid is returned, and also - * *detached is set true. + * If the partition is being detached, *detached is set true (but the original + * parent is still returned.) */ static Oid get_partition_parent_worker(Relation inhRel, Oid relid, bool *detached) @@ -106,12 +106,9 @@ get_partition_parent_worker(Relation inhRel, Oid relid, bool *detached) /* A partition being detached has no parent */ if (form->inhdetachpending) - { *detached = true; - result = InvalidOid; - } - else - result = form->inhparent; + + result = form->inhparent; } systable_endscan(scan); @@ -154,9 +151,12 @@ get_partition_ancestors_worker(Relation inhRel, Oid relid, List **ancestors) Oid parentOid; bool detached; - /* Recursion ends at the topmost level, ie., when there's no parent */ + /* + * Recursion ends at the topmost level, ie., when there's no parent; also + * when the partition is being detached. + */ parentOid = get_partition_parent_worker(inhRel, relid, &detached); - if (parentOid == InvalidOid) + if (parentOid == InvalidOid || detached) return; *ancestors = lappend_oid(*ancestors, parentOid); @@ -189,7 +189,7 @@ index_get_partition(Relation partition, Oid indexId) ReleaseSysCache(tup); if (!ispartition) continue; - if (get_partition_parent(partIdx) == indexId) + if (get_partition_parent(partIdx, false) == indexId) { list_free(idxlist); return partIdx; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 21dc869b0b..853dc8d257 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1547,7 +1547,7 @@ RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid, */ if (is_partition && relOid != oldRelOid) { - state->partParentOid = get_partition_parent(relOid); + state->partParentOid = get_partition_parent(relOid, false); if (OidIsValid(state->partParentOid)) LockRelationOid(state->partParentOid, AccessExclusiveLock); } @@ -6816,7 +6816,7 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode) /* If rel is partition, shouldn't drop NOT NULL if parent has the same */ if (rel->rd_rel->relispartition) { - Oid parentId = get_partition_parent(RelationGetRelid(rel)); + Oid parentId = get_partition_parent(RelationGetRelid(rel), false); Relation parent = table_open(parentId, AccessShareLock); TupleDesc tupDesc = RelationGetDescr(parent); AttrNumber parent_attnum; @@ -17361,7 +17361,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, if (!has_superclass(idxid)) continue; - Assert((IndexGetRelation(get_partition_parent(idxid), false) == + Assert((IndexGetRelation(get_partition_parent(idxid, false), false) == RelationGetRelid(rel))); idx = index_open(idxid, AccessExclusiveLock); @@ -17660,7 +17660,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) /* Silently do nothing if already in the right state */ currParent = partIdx->rd_rel->relispartition ? - get_partition_parent(partIdxId) : InvalidOid; + get_partition_parent(partIdxId, false) : InvalidOid; if (currParent != RelationGetRelid(parentIdx)) { IndexInfo *childInfo; @@ -17872,8 +17872,8 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl) /* make sure we see the validation we just did */ CommandCounterIncrement(); - parentIdxId = get_partition_parent(RelationGetRelid(partedIdx)); - parentTblId = get_partition_parent(RelationGetRelid(partedTbl)); + parentIdxId = get_partition_parent(RelationGetRelid(partedIdx), false); + parentTblId = get_partition_parent(RelationGetRelid(partedTbl), false); parentIdx = relation_open(parentIdxId, AccessExclusiveLock); parentTbl = relation_open(parentTblId, AccessExclusiveLock); Assert(!parentIdx->rd_index->indisvalid); diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index 0fe4f55b04..6dfa3fb4a8 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -352,16 +352,9 @@ generate_partition_qual(Relation rel) return copyObject(rel->rd_partcheck); /* - * Obtain parent relid; if it's invalid, then the partition is being - * detached. The constraint is NIL in that case, and let's cache that. + * Obtain parent relid. XXX explain why we need this */ - parentrelid = get_partition_parent(RelationGetRelid(rel)); - if (parentrelid == InvalidOid) - { - rel->rd_partcheckvalid = true; - rel->rd_partcheck = NIL; - return NIL; - } + parentrelid = get_partition_parent(RelationGetRelid(rel), true); /* Grab at least an AccessShareLock on the parent table */ parent = relation_open(parentrelid, AccessShareLock); diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h index fe3f66befa..c8c7bc1d99 100644 --- a/src/include/catalog/partition.h +++ b/src/include/catalog/partition.h @@ -19,7 +19,7 @@ /* Seed for the extended hash function */ #define HASH_PARTITION_SEED UINT64CONST(0x7A5B22367996DCFD) -extern Oid get_partition_parent(Oid relid); +extern Oid get_partition_parent(Oid relid, bool even_if_detached); extern List *get_partition_ancestors(Oid relid); extern Oid index_get_partition(Relation partition, Oid indexId); extern List *map_partition_varattnos(List *expr, int fromrel_varno, -- 2.20.1