From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tender Wang <tndrwang@gmail.com>
Cc: Junwang Zhao <zhjwpku@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Baehler Thomas SBB CFF FFS <thomas.baehler2@sbb.ch>
Cc: Guillaume Lelarge <guillaume@lelarge.info>
Cc: Michael Paquier <michael@paquier.xyz>
Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
Date: Thu, 5 Sep 2024 00:57:28 +0200
Message-ID: <20240905005728.0836d609@karst> (raw)
In-Reply-To: <20240902230147.0d3958bc@karst>
References: <20240820102651.273ecdf5@karst>
<202408210309.d5q57gjclpcq@alvherre.pgsql>
<20240902230147.0d3958bc@karst>
On Mon, 2 Sep 2024 23:01:47 +0200
Jehan-Guillaume de Rorthais <jgdr@dalibo.com> wrote:
[…]
> My proposal was to clean everything related to the old FK and use some
> existing code path to create a fresh and cleaner one. This requires some
> refactoring in existing code, but we would win a common path of code between
> create/attach/detach, a cleaner catalog and easier code maintenance.
>
> I've finally been able to write a PoC that implement this by calling
> addFkRecurseReferenced() from DetachPartitionFinalize(). I can't join
> it here because it is currently an ugly draft and I still have some work
> to do. But I would really like to have a little more time (one or two days)
> to explore this avenue further before you commit yours, if you don't mind?
> Or maybe you already have considered this avenue and rejected it?
Please, find in attachment a patch implementing this idea.
Regards,
Attachments:
[text/x-patch] v2-0001-Rework-foreign-key-mangling-during-ATTACH-DETACH.patch (26.0K, ../20240905005728.0836d609@karst/2-v2-0001-Rework-foreign-key-mangling-during-ATTACH-DETACH.patch)
download | inline diff:
From 0a235cc7040e9b33ba0bb03b0cff34a7281db137 Mon Sep 17 00:00:00 2001
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Date: Wed, 5 Jul 2023 19:19:40 +0200
Subject: [PATCH] Rework foreign key mangling during ATTACH/DETACH
---
src/backend/commands/tablecmds.c | 363 +++++++++++++++++-----
src/test/regress/expected/foreign_key.out | 67 ++++
src/test/regress/sql/foreign_key.sql | 40 +++
3 files changed, 386 insertions(+), 84 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex b3cc6f8f69..f345d6f018 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -504,7 +504,13 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *
Relation rel, Constraint *fkconstraint,
bool recurse, bool recursing,
LOCKMODE lockmode);
-static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint,+static ObjectAddress addFkConstraint(Constraint *fkconstraint, Relation rel,+ Relation pkrel, Oid indexOid, Oid parentConstr,+ int numfks, int16 *pkattnum, int16 *fkattnum,+ Oid *pfeqoperators, Oid *ppeqoperators,+ Oid *ffeqoperators, int numfkdelsetcols,+ int16 *fkdelsetcols);+static void addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint,
Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr,
int numfks, int16 *pkattnum, int16 *fkattnum,
Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators,
@@ -645,9 +651,11 @@ static void DropClonedTriggersFromPartition(Oid partitionId);
static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab,
Relation rel, RangeVar *name,
bool concurrent);
-static void DetachPartitionFinalize(Relation rel, Relation partRel,- bool concurrent, Oid defaultPartOid);-static ObjectAddress ATExecDetachPartitionFinalize(Relation rel, RangeVar *name);+static void DetachPartitionFinalize(List **wqueue, Relation rel,+ Relation partRel, bool concurrent,+ Oid defaultPartOid);+static ObjectAddress ATExecDetachPartitionFinalize(List **wqueue, Relation rel,+ RangeVar *name);
static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx,
RangeVar *name);
static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl);
@@ -5468,7 +5476,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
((PartitionCmd *) cmd->def)->concurrent);
break;
case AT_DetachPartitionFinalize:
- address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name);+ address = ATExecDetachPartitionFinalize(wqueue, rel, ((PartitionCmd *) cmd->def)->name);
break;
default: /* oops */
elog(ERROR, "unrecognized alter table type: %d",
@@ -9901,7 +9909,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
* Create all the constraint and trigger objects, recursing to partitions
* as necessary. First handle the referenced side.
*/
- address = addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel,+ address = addFkConstraint(fkconstraint, rel, pkrel,
indexOid,
InvalidOid, /* no parent constraint */
numfks,
@@ -9911,6 +9919,18 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
ppeqoperators,
ffeqoperators,
numfkdelsetcols,
+ fkdelsetcols);++ addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel,+ indexOid,+ address.objectId,+ numfks,+ pkattnum,+ fkattnum,+ pfeqoperators,+ ppeqoperators,+ ffeqoperators,+ numfkdelsetcols,
fkdelsetcols,
old_check_ok,
InvalidOid, InvalidOid);
@@ -9974,47 +9994,20 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums,
}
}
-/*- * addFkRecurseReferenced- * subroutine for ATAddForeignKeyConstraint; recurses on the referenced- * side of the constraint- *- * Create pg_constraint rows for the referenced side of the constraint,- * referencing the parent of the referencing side; also create action triggers- * on leaf partitions. If the table is partitioned, recurse to handle each- * partition.- *- * wqueue is the ALTER TABLE work queue; can be NULL when not running as part- * of an ALTER TABLE sequence.- * fkconstraint is the constraint being added.- * rel is the root referencing relation.- * pkrel is the referenced relation; might be a partition, if recursing.- * indexOid is the OID of the index (on pkrel) implementing this constraint.- * parentConstr is the OID of a parent constraint; InvalidOid if this is a- * top-level constraint.- * numfks is the number of columns in the foreign key- * pkattnum is the attnum array of referenced attributes.- * fkattnum is the attnum array of referencing attributes.- * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT- * (...) clause- * fkdelsetcols is the attnum array of the columns in the ON DELETE SET- * NULL/DEFAULT clause- * pf/pp/ffeqoperators are OID array of operators between columns.- * old_check_ok signals that this constraint replaces an existing one that- * was already validated (thus this one doesn't need validation).- * parentDelTrigger and parentUpdTrigger, when being recursively called on- * a partition, are the OIDs of the parent action triggers for DELETE and- * UPDATE respectively.- */
static ObjectAddress
-addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,- Relation pkrel, Oid indexOid, Oid parentConstr,- int numfks,- int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators,- Oid *ppeqoperators, Oid *ffeqoperators,- int numfkdelsetcols, int16 *fkdelsetcols,- bool old_check_ok,- Oid parentDelTrigger, Oid parentUpdTrigger)+addFkConstraint(Constraint *fkconstraint,+ Relation rel,+ Relation pkrel,+ Oid indexOid,+ Oid parentConstr,+ int numfks,+ int16 *pkattnum,+ int16 *fkattnum,+ Oid *pfeqoperators,+ Oid *ppeqoperators,+ Oid *ffeqoperators,+ int numfkdelsetcols,+ int16 *fkdelsetcols)
{
ObjectAddress address;
Oid constrOid;
@@ -10022,8 +10015,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
bool conislocal;
int coninhcount;
bool connoinherit;
- Oid deleteTriggerOid,- updateTriggerOid;
/*
* Verify relkind for each referenced partition. At the top level, this
@@ -10121,12 +10112,60 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
/* make new constraint visible, in case we add more */
CommandCounterIncrement();
+ return address;+}++/*+ * addFkRecurseReferenced+ * subroutine for ATAddForeignKeyConstraint; recurses on the referenced+ * side of the constraint+ *+ * Create pg_constraint rows for the referenced side of the constraint,+ * referencing the parent of the referencing side; also create action triggers+ * on leaf partitions. If the table is partitioned, recurse to handle each+ * partition.+ *+ * wqueue is the ALTER TABLE work queue; can be NULL when not running as part+ * of an ALTER TABLE sequence.+ * fkconstraint is the constraint being added.+ * rel is the root referencing relation.+ * pkrel is the referenced relation; might be a partition, if recursing.+ * indexOid is the OID of the index (on pkrel) implementing this constraint.+ * parentConstr is the OID of a parent constraint; InvalidOid if this is a+ * top-level constraint.+ * numfks is the number of columns in the foreign key+ * pkattnum is the attnum array of referenced attributes.+ * fkattnum is the attnum array of referencing attributes.+ * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT+ * (...) clause+ * fkdelsetcols is the attnum array of the columns in the ON DELETE SET+ * NULL/DEFAULT clause+ * pf/pp/ffeqoperators are OID array of operators between columns.+ * old_check_ok signals that this constraint replaces an existing one that+ * was already validated (thus this one doesn't need validation).+ * parentDelTrigger and parentUpdTrigger, when being recursively called on+ * a partition, are the OIDs of the parent action triggers for DELETE and+ * UPDATE respectively.+ */+static void+addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,+ Relation pkrel, Oid indexOid, Oid parentConstr,+ int numfks,+ int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators,+ Oid *ppeqoperators, Oid *ffeqoperators,+ int numfkdelsetcols, int16 *fkdelsetcols,+ bool old_check_ok,+ Oid parentDelTrigger, Oid parentUpdTrigger)+{+ Oid deleteTriggerOid,+ updateTriggerOid;+
/*
* Create the action triggers that enforce the constraint.
*/
createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel),
fkconstraint,
- constrOid, indexOid,+ parentConstr, indexOid,
parentDelTrigger, parentUpdTrigger,
&deleteTriggerOid, &updateTriggerOid);
@@ -10145,6 +10184,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
AttrMap *map;
AttrNumber *mapped_pkattnum;
Oid partIndexId;
+ ObjectAddress address;
partRel = table_open(pd->oids[i], ShareRowExclusiveLock);
@@ -10169,8 +10209,14 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
if (!OidIsValid(partIndexId))
elog(ERROR, "index for %u not found in partition %s",
indexOid, RelationGetRelationName(partRel));
++ address = addFkConstraint(fkconstraint, rel, partRel,+ partIndexId, parentConstr, numfks,+ mapped_pkattnum, fkattnum,+ pfeqoperators, ppeqoperators, ffeqoperators,+ numfkdelsetcols, fkdelsetcols);
addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel,
- partIndexId, constrOid, numfks,+ partIndexId, address.objectId, numfks,
mapped_pkattnum, fkattnum,
pfeqoperators, ppeqoperators, ffeqoperators,
numfkdelsetcols, fkdelsetcols,
@@ -10186,8 +10232,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel,
}
}
}
-- return address;
}
/*
@@ -10549,6 +10593,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
int numfkdelsetcols;
AttrNumber confdelsetcols[INDEX_MAX_KEYS];
Constraint *fkconstraint;
+ ObjectAddress address;
Oid deleteTriggerOid,
updateTriggerOid;
@@ -10582,7 +10627,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
* Because we're only expanding the key space at the referenced side,
* we don't need to prevent any operation in the referencing table, so
* AccessShareLock suffices (assumes that dropping the constraint
- * acquires AEL).+ * acquires AccessExclusiveLock).
*/
fkRel = table_open(constrForm->conrelid, AccessShareLock);
@@ -10648,12 +10693,16 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel)
constrForm->confrelid, constrForm->conrelid,
&deleteTriggerOid, &updateTriggerOid);
+ address = addFkConstraint(fkconstraint, fkRel, partitionRel, partIndexId,+ constrOid, numfks, mapped_confkey, conkey,+ conpfeqop, conppeqop, conffeqop,+ numfkdelsetcols, confdelsetcols);
addFkRecurseReferenced(NULL,
fkconstraint,
fkRel,
partitionRel,
partIndexId,
- constrOid,+ address.objectId,
numfks,
mapped_confkey,
conkey,
@@ -11105,6 +11154,83 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger,
partRelid);
+ /*+ * If the referenced table is partitioned, then the partition we're+ * attaching now has extra pg_constraint rows and action triggers that are+ * no longer needed. Remove those.+ */+ if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE)+ {+ Relation pg_constraint = table_open(ConstraintRelationId, RowShareLock);+ ObjectAddresses *objs;+ HeapTuple consttup;++ ScanKeyInit(&key,+ Anum_pg_constraint_conrelid,+ BTEqualStrategyNumber, F_OIDEQ,+ ObjectIdGetDatum(fk->conrelid));++ scan = systable_beginscan(pg_constraint,+ ConstraintRelidTypidNameIndexId,+ true, NULL, 1, &key);+ objs = new_object_addresses();+ while ((consttup = systable_getnext(scan)) != NULL)+ {+ Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup);++ if (conform->conparentid != fk->conoid)+ continue;+ else+ {+ ObjectAddress addr;+ int n;+ SysScanDesc scan2;+ ScanKeyData key2;++ ObjectAddressSet(addr, ConstraintRelationId, conform->oid);+ add_exact_object_address(&addr, objs);++ /*+ * First we must delete the dependency records that bind+ * the constraint records together.+ */+ n = deleteDependencyRecordsForSpecific(ConstraintRelationId,+ conform->oid,+ DEPENDENCY_INTERNAL,+ ConstraintRelationId,+ fk->conoid);+ if (n != 1)+ elog(WARNING, "oops: found %d instead of 1 deps from %u to %u",+ n, conform->oid, fk->conoid);++ /*+ * Now search for the triggers for this constraint and set+ * them up for deletion too+ */+ ScanKeyInit(&key2,+ Anum_pg_trigger_tgconstraint,+ BTEqualStrategyNumber, F_OIDEQ,+ ObjectIdGetDatum(conform->oid));+ scan2 = systable_beginscan(trigrel, TriggerConstraintIndexId,+ true, NULL, 1, &key2);+ while ((trigtup = systable_getnext(scan2)) != NULL)+ {+ ObjectAddressSet(addr, TriggerRelationId,+ ((Form_pg_trigger) GETSTRUCT(trigtup))->oid);+ add_exact_object_address(&addr, objs);+ }+ systable_endscan(scan2);+ }+ }+ /* make the dependency deletions visible */+ CommandCounterIncrement();+ performMultipleDeletions(objs, DROP_RESTRICT,+ PERFORM_DELETION_INTERNAL);+ systable_endscan(scan);++ table_close(pg_constraint, RowShareLock);+ }+
CommandCounterIncrement();
return true;
}
@@ -19094,7 +19220,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
}
/* Do the final part of detaching */
- DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid);+ DetachPartitionFinalize(wqueue, rel, partRel, concurrent, defaultPartOid);
ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel));
@@ -19111,8 +19237,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
* transaction of the concurrent algorithm fails (crash or abort).
*/
static void
-DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,- Oid defaultPartOid)+DetachPartitionFinalize(List **wqueue, Relation rel, Relation partRel,+ bool concurrent, Oid defaultPartOid)
{
Relation classRel;
List *fks;
@@ -19148,8 +19274,11 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
{
ForeignKeyCacheInfo *fk = lfirst(cell);
HeapTuple contup;
+ HeapTuple parentConTup;
Form_pg_constraint conform;
+ Form_pg_constraint parentConForm;
Constraint *fkconstraint;
+ Oid parentConstrOid;
Oid insertTriggerOid,
updateTriggerOid;
@@ -19166,7 +19295,20 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
continue;
}
- /* unset conparentid and adjust conislocal, coninhcount, etc. */+ parentConstrOid = conform->conparentid;++ Assert(OidIsValid(conform->conparentid));+ parentConTup = SearchSysCache1(CONSTROID,+ ObjectIdGetDatum(parentConstrOid));+ if (!HeapTupleIsValid(parentConTup))+ elog(ERROR, "cache lookup failed for constraint %u",+ conform->conparentid);+ parentConForm = (Form_pg_constraint) GETSTRUCT(parentConTup);++ /*+ * The constraint on this table must be marked no longer a child of+ * the parent's constraint, as do its check triggers.+ */
ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid);
/*
@@ -19184,35 +19326,88 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
RelationGetRelid(partRel));
/*
- * 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 referenced side is partitioned (which we know because our+ * parent's constraint points to a different relation than ours) then+ * we must, in addition to the above, create pg_constraint rows that+ * point to each partition, each with its own action triggers.
*/
- 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->fk_del_set_cols = NIL;- fkconstraint->old_conpfeqop = NIL;- fkconstraint->old_pktable_oid = InvalidOid;- fkconstraint->skip_validation = false;- fkconstraint->initially_valid = true;+ if (parentConForm->conrelid != conform->conrelid)+ {+ int numfks;+ AttrNumber conkey[INDEX_MAX_KEYS];+ AttrMap *attmap;+ AttrNumber confkey[INDEX_MAX_KEYS];+ Oid conpfeqop[INDEX_MAX_KEYS];+ Oid conppeqop[INDEX_MAX_KEYS];+ Oid conffeqop[INDEX_MAX_KEYS];+ int numfkdelsetcols;+ AttrNumber confdelsetcols[INDEX_MAX_KEYS];+ Relation refdRel;++ DeconstructFkConstraintRow(contup,+ &numfks,+ conkey,+ confkey,+ conpfeqop,+ conppeqop,+ conffeqop,+ &numfkdelsetcols,+ confdelsetcols);++ /* Create a synthetic node we'll use throughout */+ fkconstraint = makeNode(Constraint);+ fkconstraint->contype = CONSTRAINT_FOREIGN;+ fkconstraint->conname = pstrdup(NameStr(conform->conname));+ fkconstraint->deferrable = conform->condeferrable;+ fkconstraint->initdeferred = conform->condeferred;+ fkconstraint->skip_validation = true;+ fkconstraint->initially_valid = true;+ /* a few irrelevant fields omitted here */+ 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->fk_del_set_cols = NIL;+ fkconstraint->old_conpfeqop = NIL;+ fkconstraint->old_pktable_oid = InvalidOid;+ fkconstraint->location = -1;++ attmap = build_attrmap_by_name(RelationGetDescr(partRel),+ RelationGetDescr(rel),+ false);+ for (int i = 0; i < numfks; i++)+ {+ Form_pg_attribute att;- createForeignKeyActionTriggers(partRel, conform->confrelid,- fkconstraint, fk->conoid,+ att = TupleDescAttr(RelationGetDescr(partRel),+ attmap->attnums[conkey[i] - 1] - 1);+ fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs,+ makeString(NameStr(att->attname)));+ }++ refdRel = table_open(fk->confrelid, AccessShareLock);++ addFkRecurseReferenced(wqueue, fkconstraint, partRel,+ refdRel,
conform->conindid,
- InvalidOid, InvalidOid,- NULL, NULL);+ fk->conoid,+ numfks,+ confkey,+ conkey,+ conpfeqop,+ conppeqop,+ conffeqop,+ numfkdelsetcols,+ confdelsetcols,+ true,+ InvalidOid, InvalidOid);+ table_close(refdRel, AccessShareLock);+ }
ReleaseSysCache(contup);
+ ReleaseSysCache(parentConTup);
}
list_free_deep(fks);
if (trigrel)
@@ -19359,7 +19554,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
* completion; this completes the detaching process.
*/
static ObjectAddress
-ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)+ATExecDetachPartitionFinalize(List **wqueue, Relation rel, RangeVar *name)
{
Relation partRel;
ObjectAddress address;
@@ -19377,7 +19572,7 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)
*/
WaitForOlderSnapshots(snap->xmin, false);
- DetachPartitionFinalize(rel, partRel, true, InvalidOid);+ DetachPartitionFinalize(wqueue, rel, partRel, true, InvalidOid);
ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel));
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.outindex 8c04a24b37..43ce77a835 100644--- a/src/test/regress/expected/foreign_key.out+++ b/src/test/regress/expected/foreign_key.out@@ -2934,3 +2934,70 @@ DETAIL: drop cascades to table fkpart11.pk
drop cascades to table fkpart11.fk_parted
drop cascades to table fkpart11.fk_another
drop cascades to function fkpart11.print_row()
+-- When a table is attached as partition to a partitioned table that has+-- a foreign key to another partitioned table, it acquires a clone of the+-- FK. Upon detach, this clone is not removed, but instead becomes an+-- independent FK. If it then attaches to the partitioned table again,+-- the FK from the parent "takes over" ownership of the independent FK rather+-- than creating a separate one.+CREATE SCHEMA fkpart12+ CREATE TABLE fk_p ( id bigint PRIMARY KEY ) PARTITION BY list (id)+ CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1)+ CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2)+ CREATE TABLE fk_r_1 ( id bigint PRIMARY KEY, p_id bigint NOT NULL)+ CREATE TABLE fk_r_2 ( id bigint PRIMARY KEY, p_id bigint NOT NULL)+ CREATE TABLE fk_r ( id bigint PRIMARY KEY, p_id bigint NOT NULL,+ FOREIGN KEY (p_id) REFERENCES fk_p (id)+ ) PARTITION BY list (id);+SET search_path TO fkpart12;+INSERT INTO fk_p VALUES (1);+ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2);+ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);+\d fk_r_1+ Table "fkpart12.fk_r_1"+ Column | Type | Collation | Nullable | Default +--------+--------+-----------+----------+---------+ id | bigint | | not null | + p_id | bigint | | not null | +Partition of: fk_r FOR VALUES IN ('1')+Indexes:+ "fk_r_1_pkey" PRIMARY KEY, btree (id)+Foreign-key constraints:+ TABLE "fk_r" CONSTRAINT "fk_r_p_id_fkey" FOREIGN KEY (p_id) REFERENCES fk_p(id)++INSERT INTO fk_r VALUES (1,1);+ALTER TABLE fk_r DETACH PARTITION fk_r_1;+\d fk_r_1+ Table "fkpart12.fk_r_1"+ Column | Type | Collation | Nullable | Default +--------+--------+-----------+----------+---------+ id | bigint | | not null | + p_id | bigint | | not null | +Indexes:+ "fk_r_1_pkey" PRIMARY KEY, btree (id)+Foreign-key constraints:+ "fk_r_p_id_fkey" FOREIGN KEY (p_id) REFERENCES fk_p(id)++INSERT INTO fk_r_1 VALUES (2,2); -- fails as EXPECTED+ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_fkey"+DETAIL: Key (p_id)=(2) is not present in table "fk_p".+DELETE FROM fk_p; -- should fails but was buggy+ERROR: update or delete on table "fk_p_1" violates foreign key constraint "fk_r_1_p_id_fkey" on table "fk_r_1"+DETAIL: Key (id)=(1) is still referenced from table "fk_r_1".+ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);+\d fk_r_1+ Table "fkpart12.fk_r_1"+ Column | Type | Collation | Nullable | Default +--------+--------+-----------+----------+---------+ id | bigint | | not null | + p_id | bigint | | not null | +Partition of: fk_r FOR VALUES IN ('1')+Indexes:+ "fk_r_1_pkey" PRIMARY KEY, btree (id)+Foreign-key constraints:+ TABLE "fk_r" CONSTRAINT "fk_r_p_id_fkey" FOREIGN KEY (p_id) REFERENCES fk_p(id)++SET client_min_messages TO warning;+DROP SCHEMA fkpart12 CASCADE;+RESET client_min_messages;+RESET search_path;diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sqlindex d1aac5357f..5d8d2558c0 100644--- a/src/test/regress/sql/foreign_key.sql+++ b/src/test/regress/sql/foreign_key.sql@@ -2086,3 +2086,43 @@ UPDATE fkpart11.pk SET a = 3 WHERE a = 4;
UPDATE fkpart11.pk SET a = 1 WHERE a = 2;
DROP SCHEMA fkpart11 CASCADE;
++-- When a table is attached as partition to a partitioned table that has+-- a foreign key to another partitioned table, it acquires a clone of the+-- FK. Upon detach, this clone is not removed, but instead becomes an+-- independent FK. If it then attaches to the partitioned table again,+-- the FK from the parent "takes over" ownership of the independent FK rather+-- than creating a separate one.+CREATE SCHEMA fkpart12+ CREATE TABLE fk_p ( id bigint PRIMARY KEY ) PARTITION BY list (id)+ CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1)+ CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2)+ CREATE TABLE fk_r_1 ( id bigint PRIMARY KEY, p_id bigint NOT NULL)+ CREATE TABLE fk_r_2 ( id bigint PRIMARY KEY, p_id bigint NOT NULL)+ CREATE TABLE fk_r ( id bigint PRIMARY KEY, p_id bigint NOT NULL,+ FOREIGN KEY (p_id) REFERENCES fk_p (id)+ ) PARTITION BY list (id);+SET search_path TO fkpart12;++INSERT INTO fk_p VALUES (1);++ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2);++ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);+\d fk_r_1++INSERT INTO fk_r VALUES (1,1);++ALTER TABLE fk_r DETACH PARTITION fk_r_1;+\d fk_r_1++INSERT INTO fk_r_1 VALUES (2,2); -- fails as EXPECTED+DELETE FROM fk_p; -- should fails but was buggy++ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1);+\d fk_r_1++SET client_min_messages TO warning;+DROP SCHEMA fkpart12 CASCADE;+RESET client_min_messages;+RESET search_path;--
2.46.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: jgdr@dalibo.com, alvherre@alvh.no-ip.org, tndrwang@gmail.com, zhjwpku@gmail.com, exclusion@gmail.com, thomas.baehler2@sbb.ch, guillaume@lelarge.info, michael@paquier.xyz
Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails
In-Reply-To: <20240905005728.0836d609@karst>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox