From 4ae09c59d6f563531c456eda3147c849773ceaa7 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 12 Jul 2024 12:54:01 +0200 Subject: Fix ALTER TABLE DETACH for inconsistent indexes When a partitioned table has an index that doesn't support a constraint, but a partition has an equivalent index that does, then a DETACH operation would misbehave: a crash in assertion-enabled systems (because we fail to find the constraint in the parent that we expect to), or a broken coninhcount value (-1) in production systems (because we blindly believe that we've successfully detached the parent). While we should reject an ATTACH of a partition with such an index, we have failed to do so in existing releases, so adding an error in stable releases might break the (unlikely) existing applications that rely on this behavior. At this point I don't even want to reject them in master, because it'd break pg_upgrade if such databases exist, and there would be no easy way to fix existing databases without expensive index rebuilds. (Later on we could add ALTER TABLE ... ADD CONSTRAINT USING INDEX to partitioned tables, which would allow the user to fix such patterns. At that point we could add more restrictions to prevent the problem from its root.) Also, add a test case that leaves one table in this condition, so that we can verify that pg_upgrade continues to work if we later decide to change the policy on the master branch. Backpatch to all supported branches. Co-authored-by: Tender Wang Reported-by: Alexander Lakhin Reviewed-by: Tender Wang Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/18500-62948b6fe5522f56@postgresql.org --- src/backend/commands/tablecmds.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src/backend/commands/tablecmds.c') diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b393bd9497f..8e3e3919f75 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -18782,22 +18782,31 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, foreach(cell, indexes) { Oid idxid = lfirst_oid(cell); + Oid parentidx; Relation idx; Oid constrOid; + Oid parentConstrOid; if (!has_superclass(idxid)) continue; - Assert((IndexGetRelation(get_partition_parent(idxid, false), false) == - RelationGetRelid(rel))); + parentidx = get_partition_parent(idxid, false); + Assert((IndexGetRelation(parentidx, false) == RelationGetRelid(rel))); idx = index_open(idxid, AccessExclusiveLock); IndexSetParentIndex(idx, InvalidOid); - /* If there's a constraint associated with the index, detach it too */ + /* + * If there's a constraint associated with the index, detach it too. + * Careful: it is possible for a constraint index in a partition to be + * the child of a non-constraint index, so verify whether the parent + * index does actually have a constraint. + */ constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel), idxid); - if (OidIsValid(constrOid)) + parentConstrOid = get_relation_idx_constraint_oid(RelationGetRelid(rel), + parentidx); + if (OidIsValid(parentConstrOid) && OidIsValid(constrOid)) ConstraintSetParentConstraint(constrOid, InvalidOid, InvalidOid); index_close(idx, NoLock); -- cgit v1.2.3