database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-09-03 15:22:11 +03:00
Code Activity

Restructure foreign key handling code for ATTACH/DETACH

Browse Source 
... to fix bugs when the referenced table is partitioned.

The catalog representation we chose for foreign keys connecting
partitioned tables (in commit f56f8f8da6) is inconvenient, in the
sense that a standalone table has a different way to represent the
constraint when referencing a partitioned table, than when the same
table becomes a partition (and vice versa).  Because of this, we need to
create additional catalog rows on detach (pg_constraint and pg_trigger),
and remove them on attach.  We were doing some of those things, but not
all of them, leading to missing catalog rows in certain cases.

The worst problem seems to be that we are missing action triggers after
detaching a partition, which means that you could update/delete rows
from the referenced partitioned table that still had referencing rows on
that table, the server failing to throw the required errors.

!!!
Note that this means existing databases with FKs that reference
partitioned tables might have rows that break relational integrity, on
tables that were once partitions on the referencing side of the FK.

Another possible problem is that trying to reattach a table
that had been detached would fail indicating that internal triggers
cannot be found, which from the user's point of view is nonsensical.

In branches 15 and above, we fix this by creating a new helper function
addFkConstraint() which is in charge of creating a standalone
pg_constraint row, and repurposing addFkRecurseReferencing() and
addFkRecurseReferenced() so that they're only the recursive routine for
each side of the FK, and they call addFkConstraint() to create
pg_constraint at each partitioning level and add the necessary triggers.
These new routines can be used during partition creation, partition
attach and detach, and foreign key creation.  This reduces redundant
code and simplifies the flow.

In branches 14 and 13, we have a much simpler fix that consists on
simply removing the constraint on detach.  The reason is that those
branches are missing commit f4566345cf, which reworked the way this
works in a way that we didn't consider back-patchable at the time.

We opted to leave branch 12 alone, because it's different from branch 13
enough that the fix doesn't apply; and because it is going in EOL mode
very soon, patching it now might be worse since there's no way to undo
the damage if it goes wrong.

Existing databases might need to be repaired.

In the future we might want to rethink the catalog representation to
avoid this problem, but for now the code seems to do what's required to
make the constraints operate correctly.

Co-authored-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Co-authored-by: Tender Wang <tndrwang@gmail.com>
Co-authored-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Guillaume Lelarge <guillaume@lelarge.info>
Reported-by: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Reported-by: Thomas Baehler (SBB CFF FFS) <thomas.baehler2@sbb.ch>
Discussion: https://postgr.es/m/20230420144344.40744130@karst
Discussion: https://postgr.es/m/20230705233028.2f554f73@karst
Discussion: https://postgr.es/m/GVAP278MB02787E7134FD691861635A8BC9032@GVAP278MB0278.CHEP278.PROD.OUTLOOK.COM
Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org
This commit is contained in:
Álvaro Herrera
2024-10-22 16:01:18 +02:00
parent beab395a42
commit d20194cead
3 changed files with 281 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

160
src/backend/commands/tablecmds.c
View File

@@ -10102,6 +10102,82 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
table_close(trigrel, RowExclusiveLock);
ConstraintSetParentConstraint(fk->conoid, parentConstrOid, 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;
SysScanDesc scan2;
ScanKeyData key2;
int n PG_USED_FOR_ASSERTS_ONLY;
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);
Assert(n == 1); /* actually only one is expected */
/*
* 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;
}
@@ -17470,6 +17546,7 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
List *indexes;
List *fks;
ListCell *cell;
ObjectAddresses *dropobjs = NULL;
/*
* We must lock the default partition, because detaching this partition
@@ -17567,8 +17644,8 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
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)
@@ -17576,7 +17653,6 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
ForeignKeyCacheInfo *fk = lfirst(cell);
HeapTuple contup;
Form_pg_constraint conform;
Constraint *fkconstraint;
contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
if (!HeapTupleIsValid(contup))
@@ -17591,39 +17667,71 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
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();
/*
* Remember the constraint and its triggers for later deletion.
*/
if (dropobjs == NULL)
dropobjs = new_object_addresses();
ObjectAddressSet(constraddr, ConstraintRelationId, fk->conoid);
add_exact_object_address(&constraddr, dropobjs);
}
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);
}
list_free_deep(fks);
/* If we collected any constraints for deletion, do so now. */
if (dropobjs != NULL)
performMultipleDeletions(dropobjs, DROP_CASCADE, 0);
/*
* Any sub-constraints that are in the referenced-side of a larger
* constraint have to be removed. This partition is no longer part of the