On 2024-Aug-19, Alvaro Herrera wrote:
> I haven't pushed it yet, mostly because of being unsure about not doing
> anything for the oldest branches (14 and back).
Last night, after much mulling on this, it occurred to me that one easy
way out of this problem for the old branches, without having to write
more code, is to simply remove the constraint from the partition when
it's detached (but only if they reference a partitioned relation). It's
not a great solution, but at least we're no longer leaving bogus catalog
entries around. That would be like the attached patch, which was cut
from 14 and applies cleanly to 12 and 13. I'd throw in a couple of
tests and call it a day.
(TBH the idea of leaving the partition without a foreign key feels to me
like travelling in a car without a seat belt -- it feels instinctively
dangerous. This is why I went such lengths to keep FKs on detach
initially. But I'm not inclined to spend more time on this issue.
However ... what about fixing catalog content that's already broken
after past detach ops?)
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
>From e95fde451ee73b89f1a5ba50b7f2e34f5fb98bde Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@alvh.no-ip.org>
Date: Wed, 21 Aug 2024 17:47:03 -0400
Subject: [PATCH v14] drop constraint when detaching
---
src/backend/commands/tablecmds.c | 79 +++++++++++++++++++++-----------
1 file changed, 53 insertions(+), 26 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c39e6799ca..7ef904f63e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -18143,8 +18143,8 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
DropClonedTriggersFromPartition(RelationGetRelid(partRel));
/*
- * Detach any foreign keys that are inherited. This includes creating
- * additional action triggers.
+ * Detach any foreign keys that are inherited -- or, if they reference
+ * partitioned tables, drop them.
*/
fks = copyObject(RelationGetFKeyList(partRel));
foreach(cell, fks)
@@ -18152,7 +18152,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
ForeignKeyCacheInfo *fk = lfirst(cell);
HeapTuple contup;
Form_pg_constraint conform;
- Constraint *fkconstraint;
contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
if (!HeapTupleIsValid(contup))
@@ -18167,34 +18166,62 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
continue;
}
- /* unset conparentid and adjust conislocal, coninhcount, etc. */
+ /* Mark the constraint as independent */
ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
/*
- * Make the action triggers on the referenced relation. When this was
- * a partition the action triggers pointed to the parent rel (they
- * still do), but now we need separate ones of our own.
+ * If the constraint references a partitioned table, just drop the
+ * constraint, because it's more work to preserve the constraint
+ * correctly.
+ *
+ * If it references a plain table, then we can create the action
+ * triggers and it'll be okay.
*/
- fkconstraint = makeNode(Constraint);
- fkconstraint->contype = CONSTRAINT_FOREIGN;
- fkconstraint->conname = pstrdup(NameStr(conform->conname));
- fkconstraint->deferrable = conform->condeferrable;
- fkconstraint->initdeferred = conform->condeferred;
- fkconstraint->location = -1;
- fkconstraint->pktable = NULL;
- fkconstraint->fk_attrs = NIL;
- fkconstraint->pk_attrs = NIL;
- fkconstraint->fk_matchtype = conform->confmatchtype;
- fkconstraint->fk_upd_action = conform->confupdtype;
- fkconstraint->fk_del_action = conform->confdeltype;
- fkconstraint->old_conpfeqop = NIL;
- fkconstraint->old_pktable_oid = InvalidOid;
- fkconstraint->skip_validation = false;
- fkconstraint->initially_valid = true;
+ if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE)
+ {
+ ObjectAddress constraddr;
- createForeignKeyActionTriggers(partRel, conform->confrelid,
- fkconstraint, fk->conoid,
- conform->conindid);
+ /* make the dependency deletions above visible */
+ CommandCounterIncrement();
+
+ /*
+ * And drop the constraint and its triggers; if this partition is
+ * partitioned, then the corresponding objects in the partitions
+ * too.
+ */
+ ObjectAddressSet(constraddr, ConstraintRelationId, fk->conoid);
+ performDeletion(&constraddr, DROP_CASCADE, 0);
+ }
+ else
+ {
+ Constraint *fkconstraint;
+
+ /*
+ * Make the action triggers on the referenced relation. When this
+ * was a partition the action triggers pointed to the parent rel
+ * (they still do), but now we need separate ones of our own.
+ */
+ fkconstraint = makeNode(Constraint);
+ fkconstraint->contype = CONSTRAINT_FOREIGN;
+ fkconstraint->conname = pstrdup(NameStr(conform->conname));
+ fkconstraint->deferrable = conform->condeferrable;
+ fkconstraint->initdeferred = conform->condeferred;
+ fkconstraint->location = -1;
+ fkconstraint->pktable = NULL;
+ fkconstraint->fk_attrs = NIL;
+ fkconstraint->pk_attrs = NIL;
+ fkconstraint->fk_matchtype = conform->confmatchtype;
+ fkconstraint->fk_upd_action = conform->confupdtype;
+ fkconstraint->fk_del_action = conform->confdeltype;
+ fkconstraint->old_conpfeqop = NIL;
+ fkconstraint->old_pktable_oid = InvalidOid;
+ fkconstraint->skip_validation = false;
+ fkconstraint->initially_valid = true;
+
+ createForeignKeyActionTriggers(partRel, conform->confrelid,
+ fkconstraint, fk->conoid,
+ conform->conindid);
+ }
ReleaseSysCache(contup);
}
--
2.39.2
