From 30001a544c5191d34c843a0fb24cd92e5e153c5a Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 28 Jun 2024 13:16:03 +0200 Subject: [PATCH v3 2/3] WIP: don't let constraint indexes attach to raw indexes Probably for master only, since we probably don't want to prohibit working behavior in backbranches. In backbranches we'll have to cope with the possibility that a hierarchy such as the one being forbidded here already exists. We may also want to do something about this during pg_upgrade. --- src/backend/commands/indexcmds.c | 24 ++++++++++++++++----- src/backend/commands/tablecmds.c | 37 +++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 2caab88aa5..cad12d61ec 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1378,7 +1378,11 @@ DefineIndex(Oid tableId, parentIndex->rd_opfamily, attmap)) { - Oid cldConstrOid = InvalidOid; + Oid cldConstrOid; + + cldConstrOid = + get_relation_idx_constraint_oid(childRelid, + cldidxid); /* * Found a match. @@ -1387,19 +1391,29 @@ DefineIndex(Oid tableId, * because of a constraint, then the child needs to * have a constraint also, so look for one. If there * is no such constraint, this index is no good, so - * keep looking. + * keep looking. On the other hand, if the index that + * exists on the child _is_ a constraint but this one + * isn't, then we shouldn't match them. XXX should we + * instead choose to ignore the index? */ if (createdConstraintId != InvalidOid) { - cldConstrOid = - get_relation_idx_constraint_oid(childRelid, - cldidxid); if (cldConstrOid == InvalidOid) { index_close(cldidx, lockmode); continue; } } + else if (OidIsValid(cldConstrOid)) + ereport(ERROR, + errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("can't create index on partitioned table \"%s\" matching constraint \"%s\" on partition \"%s\"", + RelationGetRelationName(rel), + get_constraint_name(cldConstrOid), + RelationGetRelationName(childrel)), + errhint("You can create the index on ONLY \"%s\", or create a constraint instead.", + RelationGetRelationName(rel))); + /* Attach index to parent and we're done. */ IndexSetParentIndex(cldidx, indexRelationId); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 37faa2a39a..4405f3b0cb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -18751,6 +18751,10 @@ AttachPartitionEnsureIndexes(List **wqueue, Relation rel, Relation attachrel) idxRel->rd_opfamily, attmap)) { + cldConstrOid = + get_relation_idx_constraint_oid(RelationGetRelid(attachrel), + cldIdxId); + /* * If this index is being created in the parent because of a * constraint, then the child needs to have a constraint also, @@ -18759,9 +18763,6 @@ AttachPartitionEnsureIndexes(List **wqueue, Relation rel, Relation attachrel) */ if (OidIsValid(constraintOid)) { - cldConstrOid = - get_relation_idx_constraint_oid(RelationGetRelid(attachrel), - cldIdxId); /* no dice */ if (!OidIsValid(cldConstrOid)) continue; @@ -18771,6 +18772,22 @@ AttachPartitionEnsureIndexes(List **wqueue, Relation rel, Relation attachrel) get_constraint_type(cldConstrOid)) continue; } + else + { + /* + * If the parent does *not* have a constraint, then the + * child must also not have one. Otherwise, things can get + * ugly by detach time. + */ + + if (OidIsValid(cldConstrOid)) + ereport(ERROR, + errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("can't attach partition \"%s\" with constraint \"%s\" that matches non-constraint index \"%s\"", + RelationGetRelationName(attachrel), + get_constraint_name(cldConstrOid), + RelationGetRelationName(idxRel))); + } /* bingo. */ IndexSetParentIndex(attachrelIdxRels[i], idx); @@ -19730,6 +19747,20 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) RelationGetRelationName(parentTbl), RelationGetRelationName(partIdx)))); } + else + { + Oid constroid; + + constroid = get_relation_idx_constraint_oid(RelationGetRelid(partTbl), + partIdxId); + + if (OidIsValid(constroid)) + ereport(ERROR, + errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("can't attach constraint-supporting index \"%s\" to non-constraint index \"%s\"", + get_constraint_name(constroid), + RelationGetRelationName(partIdx))); + } /* All good -- do it */ IndexSetParentIndex(partIdx, RelationGetRelid(parentIdx)); -- 2.39.2