From df469951f8503e4a5c00b97424c667619e3c8aba Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 28 Jun 2024 13:16:03 +0200 Subject: [PATCH v2] 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 309389e20d..50205b9bdb 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 66cda26a25..8cb9893e5d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -18757,6 +18757,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, @@ -18765,9 +18769,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; @@ -18777,6 +18778,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); @@ -19726,6 +19743,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