From d168bfa988bb8923fb138ceb4026ac005a7f580a Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 9 Jan 2019 10:00:11 +0900 Subject: [PATCH v2 1/2] Ensure PK-side action triggers for partitions after being detached Detaching a partition from the parent whose foreign key(s) would've been duplicated in the partition makes the foreign key(s) stop working corretly. That's because PK-side action trigger for the foreign key would refer to the parent as the referencing relation and after the partition is detached it's data is no longer accessible via parent. So while the check triggers that remain even after being detached takes care of the one side of enforcing the foreign key of the detached partition, lack of corresponding PK-side action trigger to check detached partition's data means the other side doesn't work. To fix, add the action triggers on the PK relation that point to the detached partition when detaching. --- src/backend/catalog/pg_constraint.c | 74 +++++++++++++++++++++++++++++-------- src/backend/commands/tablecmds.c | 42 +++++++++++++++++++-- src/include/commands/tablecmds.h | 3 +- 3 files changed, 98 insertions(+), 21 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 3c960c9423..3cac851447 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -27,6 +27,7 @@ #include "catalog/partition.h" #include "catalog/pg_constraint.h" #include "catalog/pg_operator.h" +#include "catalog/pg_trigger.h" #include "catalog/pg_type.h" #include "commands/defrem.h" #include "commands/tablecmds.h" @@ -547,20 +548,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, elog(ERROR, "conpfeqop is not a 1-D OID array"); memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); - datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop, - tupdesc, &isnull); - if (isnull) - elog(ERROR, "null conpfeqop"); - arr = DatumGetArrayTypeP(datum); - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conpfeqop is not a 1-D OID array"); - memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); - datum = fastgetattr(tuple, Anum_pg_constraint_conppeqop, tupdesc, &isnull); if (isnull) @@ -603,6 +590,11 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, ForeignKeyCacheInfo *fk = lfirst_node(ForeignKeyCacheInfo, cell); Form_pg_constraint partConstr; HeapTuple partcontup; + Relation trigrel; + HeapTuple trigtup; + Form_pg_trigger trigform; + HeapScanDesc scan; + ScanKeyData key[3]; attach_it = true; @@ -654,7 +646,56 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, ReleaseSysCache(partcontup); - /* looks good! Attach this constraint */ + /* + * looks good! Attach this constraint. Drop the action triggers + * on the PK side that refer to this partition rel as part of + * this constraint, because they will be wasteful after attaching + * this constraint to the parent constraint. After being attached, + * action triggers corresponding to the parent constraint can + * indirectly refer to this partition's data by referencing the + * parent relation. + */ + trigrel = heap_open(TriggerRelationId, AccessShareLock); + ScanKeyInit(&key[0], + Anum_pg_trigger_tgrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->confrelid)); + ScanKeyInit(&key[1], + Anum_pg_trigger_tgconstrrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conrelid)); + ScanKeyInit(&key[2], + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conoid)); + scan = heap_beginscan_catalog(trigrel, 3, key); + + /* + * There should be two tuples to be deleted corresponding to + * the two action triggers that createForeignKeyActionTriggers + * would have created. + */ + trigtup = heap_getnext(scan, ForwardScanDirection); + Assert(trigtup != NULL); + trigform = (Form_pg_trigger) GETSTRUCT(trigtup); + deleteDependencyRecordsForClass(TriggerRelationId, trigform->oid, + ConstraintRelationId, + DEPENDENCY_INTERNAL); + CatalogTupleDelete(trigrel, &trigtup->t_self); + + trigtup = heap_getnext(scan, ForwardScanDirection); + Assert(trigtup != NULL); + trigform = (Form_pg_trigger) GETSTRUCT(trigtup); + deleteDependencyRecordsForClass(TriggerRelationId, trigform->oid, + ConstraintRelationId, + DEPENDENCY_INTERNAL); + CatalogTupleDelete(trigrel, &trigtup->t_self); + + trigtup = heap_getnext(scan, ForwardScanDirection); + Assert(trigtup == NULL); + heap_endscan(scan); + heap_close(trigrel, AccessShareLock); + ConstraintSetParentConstraint(fk->conoid, constrForm->oid); CommandCounterIncrement(); attach_it = true; @@ -714,7 +755,8 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, fkconstraint->initdeferred = constrForm->condeferred; createForeignKeyTriggers(partRel, constrForm->confrelid, fkconstraint, - constrOid, constrForm->conindid, false); + constrOid, constrForm->conindid, false, + true); if (cloned) { diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d2781cbf19..80b46d3139 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7623,7 +7623,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * though the constraints also exist below. */ createForeignKeyTriggers(rel, RelationGetRelid(pkrel), fkconstraint, - constrOid, indexOid, !recursing); + constrOid, indexOid, !recursing, true); /* * Tell Phase 3 to check that the constraint is satisfied by existing @@ -8728,7 +8728,8 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid, */ void createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint, - Oid constraintOid, Oid indexOid, bool create_action) + Oid constraintOid, Oid indexOid, bool create_action, + bool create_check) { /* * For the referenced side, create action triggers, if requested. (If the @@ -8744,7 +8745,7 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint, * For the referencing side, create the check triggers. We only need * these on the partitions. */ - if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE && create_check) createForeignKeyCheckTriggers(RelationGetRelid(rel), refRelOid, fkconstraint, constraintOid, indexOid); @@ -14749,19 +14750,52 @@ ATExecDetachPartition(Relation rel, RangeVar *name) } heap_close(classRel, RowExclusiveLock); - /* Detach foreign keys */ + /* Detach any foreign keys that are inherited */ 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); + /* + * We'll need to make the action triggers on primary key relation that + * point to this relation as the FK relation. We need to do this, + * because no PK-side triggers exist for an inherited FK constraint. + * Now that we've detached it from the parent, we'd 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; + + /* + * As we already got the check triggers, no need to recreate them, + * so pass false for create_check. + */ + createForeignKeyTriggers(partRel, conform->confrelid, fkconstraint, + fk->conoid, conform->conindid, + true, false); + ReleaseSysCache(contup); } list_free_deep(fks); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index ec3bb90b01..6bebc68425 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -78,7 +78,8 @@ extern void check_of_type(HeapTuple typetuple); extern void createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint, Oid constraintOid, - Oid indexOid, bool create_action); + Oid indexOid, bool create_action, + bool create_check); extern void register_on_commit_action(Oid relid, OnCommitAction action); extern void remove_on_commit_action(Oid relid); -- 2.11.0