From 42b96db12aafea22285893e1d96c63a7f684abfd Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 18 Jan 2019 19:10:29 -0300 Subject: [PATCH 2/2] Fix action triggers for FK constraints on detached partitions --- src/backend/commands/tablecmds.c | 72 +++++++++++++++++++++++++++++++++++- src/test/regress/sql/foreign_key.sql | 20 +++++++++- 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 5a27f64aa7..02a55ed3d6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7858,6 +7858,10 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel, ForeignKeyCacheInfo *fk = lfirst_node(ForeignKeyCacheInfo, cell); Form_pg_constraint partConstr; HeapTuple partcontup; + Relation trigrel; + HeapTuple trigtup; + SysScanDesc scan; + ScanKeyData key; attach_it = true; @@ -7909,7 +7913,39 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel, ReleaseSysCache(partcontup); - /* looks good! Attach this constraint */ + /* + * Looks good! Attach this constraint. Note that the action + * triggers are no longer needed, so remove them. We identify + * them because they have our constraint OID, as well as being + * on the referenced rel. + */ + trigrel = heap_open(TriggerRelationId, RowExclusiveLock); + ScanKeyInit(&key, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conoid)); + + scan = systable_beginscan(trigrel, TriggerConstraintIndexId, true, + NULL, 1, &key); + while ((trigtup = systable_getnext(scan)) != NULL) + { + Form_pg_trigger trgform = (Form_pg_trigger) GETSTRUCT(trigtup); + + if (trgform->tgconstrrelid != fk->conrelid) + continue; + if (trgform->tgrelid != fk->confrelid) + continue; + + deleteDependencyRecordsForClass(TriggerRelationId, + trgform->oid, + ConstraintRelationId, + DEPENDENCY_INTERNAL); + CatalogTupleDelete(trigrel, &trigtup->t_self); + } + + systable_endscan(scan); + heap_close(trigrel, RowExclusiveLock); + ConstraintSetParentConstraint(fk->conoid, parentConstrOid); CommandCounterIncrement(); attach_it = true; @@ -15080,19 +15116,51 @@ ATExecDetachPartition(Relation rel, RangeVar *name) } heap_close(classRel, RowExclusiveLock); - /* Detach foreign keys */ + /* + * Detach any foreign keys that are inherited. This includes creating + * additional action triggers. + */ fks = copyObject(RelationGetFKeyList(partRel)); foreach(cell, fks) { ForeignKeyCacheInfo *fk = lfirst(cell); HeapTuple contup; + Form_pg_constraint conform; + Constraint *fkconstraint; contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid)); if (!contup) elog(ERROR, "cache lookup failed for constraint %u", fk->conoid); + conform = (Form_pg_constraint) GETSTRUCT(contup); + /* consider only the inherited foreign keys */ + if (conform->contype != CONSTRAINT_FOREIGN || + !OidIsValid(conform->conparentid)) + { + ReleaseSysCache(contup); + continue; + } + + /* unset conparentid and adjust conislocal, coninhcount, etc. */ ConstraintSetParentConstraint(fk->conoid, 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. + */ + fkconstraint = makeNode(Constraint); + fkconstraint->conname = pstrdup(NameStr(conform->conname)); + fkconstraint->fk_upd_action = conform->confupdtype; + fkconstraint->fk_del_action = conform->confdeltype; + fkconstraint->deferrable = conform->condeferrable; + fkconstraint->initdeferred = conform->condeferred; + + createForeignKeyActionTriggers(partRel, conform->confrelid, + fkconstraint, fk->conoid, + conform->conindid); + CommandCounterIncrement(); + ReleaseSysCache(contup); } list_free_deep(fks); diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index c3a7cfcc01..e08b6a21b0 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1375,6 +1375,24 @@ create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56 alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey; alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey; +-- verify that attaching and detaching partitions maintains the right set of +-- triggers +create schema fkpart1; +set search_path to fkpart1; + create table pkey (a int primary key); + create table fk_part (a int) partition by list (a); + create table fk_part_1 partition of fk_part for values in (1) partition by list (a); + create table fk_part_1_1 partition of fk_part_1 for values in (1); +alter table fkpart1.fk_part add foreign key (a) references fkpart1.pkey; +insert into fkpart1.fk_part values (1); -- should fail +insert into fkpart1.pkey values (1); +insert into fkpart1.fk_part values (1); +delete from fkpart1.pkey where a = 1; -- should fail +alter table fkpart1.fk_part detach partition fkpart1.fk_part_1; +create table fkpart1.fk_part_1_2 partition of fkpart1.fk_part_1 for values in (2); +insert into fkpart1.fk_part_1 values (2); -- should fail +delete from fkpart1.pkey where a = 1; + \set VERBOSITY terse \\ -- suppress cascade details -drop schema fkpart0 cascade; +drop schema fkpart0, fkpart1 cascade; \set VERBOSITY default -- 2.11.0