From 5b5c735589d46aee1ee1769b7663f4b041c44496 Mon Sep 17 00:00:00 2001 From: amitlan Date: Tue, 10 Nov 2020 10:54:28 +0900 Subject: [PATCH v1 1/2] Create foreign key triggers in partitioned tables too While user-defined triggers defined on a partitioned table have a catalog definition for both it and its partitions, internal triggers used by foreign keys defined on partitioned tables only have a catalog definition for its partitions. This commit fixes that so that partitioned tables get the foreign key triggers too, just like user-defined triggers. Moreover, like user-defined triggers, partitions' internal triggers will now also have their tgparentid set appropriately. This is to allow subsequent commit(s) to make the foreign key related events to be fired in some cases using the parent table triggers instead of those of partitions'. This also changes what tgisinternal means in some cases. Currently, it means either that the trigger is an internal implementation object of a foreign key constraint, or a "child" trigger on a partition cloned from the trigger on the parent. This commit changes it to only mean the former to avoid confusion. As for the latter, it can be told by tgparentid being nonzero, which is now true both for user- defined and foreign key's internal triggers. --- src/backend/commands/tablecmds.c | 354 +++++++++++++++++++++++++++------ src/backend/commands/trigger.c | 101 +++++++++- src/bin/pg_dump/pg_dump.c | 20 +- src/bin/psql/describe.c | 6 +- src/include/commands/trigger.h | 5 +- src/test/regress/expected/triggers.out | 4 +- 6 files changed, 407 insertions(+), 83 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 46f1637..5c1848c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -450,12 +450,14 @@ static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstra Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, - bool old_check_ok); + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger); static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, - bool old_check_ok, LOCKMODE lockmode); + bool old_check_ok, LOCKMODE lockmode, + Oid parentInsTrigger, Oid parentUpdTrigger); static void CloneForeignKeyConstraints(List **wqueue, Relation parentRel, Relation partitionRel); static void CloneFkReferenced(Relation parentRel, Relation partitionRel); @@ -463,15 +465,34 @@ static void CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel); static void createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, Oid constraintOid, - Oid indexOid); + Oid indexOid, + Oid parentInsTrigger, Oid parentUpdTrigger, + Oid *insertTrigOid, Oid *updateTrigOid); static void createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint, Oid constraintOid, - Oid indexOid); + Oid indexOid, + Oid parentDelTrigger, Oid parentUpdTrigger, + Oid *deleteTrigOid, Oid *updateTrigOid); static bool tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, Oid partRelid, Oid parentConstrOid, int numfks, AttrNumber *mapped_conkey, AttrNumber *confkey, - Oid *conpfeqop); + Oid *conpfeqop, + Oid parentInsTrigger, + Oid parentUpdTrigger, + Relation trigrel); +static void GetForeignKeyActionTriggers(Form_pg_constraint constrForm, + Relation trigrel, + Oid *deleteTriggerOid, + Oid *updateTriggerOid); +static void GetForeignKeyCheckTriggers(Relation trigrel, + Oid conoid, Oid confrelid, Oid conrelid, + Oid *insertTriggerOid, + Oid *updateTriggerOid); +static void AttachForeignKeyCheckTriggers(ForeignKeyCacheInfo *fk, Oid partRelid, + Oid parentInsTrigger, Oid parentUpdTrigger, + Relation trigrel); +static void DetachForeignKeyCheckTriggers(ForeignKeyCacheInfo *fk, Oid partRelid); static void ATExecDropConstraint(Relation rel, const char *constrName, DropBehavior behavior, bool recurse, bool recursing, @@ -8762,7 +8783,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, pfeqoperators, ppeqoperators, ffeqoperators, - old_check_ok); + old_check_ok, + InvalidOid, InvalidOid); /* Now handle the referencing side. */ addFkRecurseReferencing(wqueue, fkconstraint, rel, pkrel, @@ -8775,7 +8797,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ppeqoperators, ffeqoperators, old_check_ok, - lockmode); + lockmode, + InvalidOid, InvalidOid); /* * Done. Close pk table, but keep lock until we've committed. @@ -8815,7 +8838,8 @@ 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, bool old_check_ok) + Oid *ppeqoperators, Oid *ffeqoperators, bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger) { ObjectAddress address; Oid constrOid; @@ -8823,6 +8847,8 @@ 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 @@ -8922,12 +8948,11 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, * If the referenced table is a plain relation, create the action triggers * that enforce the constraint. */ - if (pkrel->rd_rel->relkind == RELKIND_RELATION) - { - createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel), - fkconstraint, - constrOid, indexOid); - } + createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel), + fkconstraint, + constrOid, indexOid, + parentDelTrigger, parentUpdTrigger, + &deleteTriggerOid, &updateTriggerOid); /* * If the referenced table is partitioned, recurse on ourselves to handle @@ -8971,7 +8996,8 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, partIndexId, constrOid, numfks, mapped_pkattnum, fkattnum, pfeqoperators, ppeqoperators, ffeqoperators, - old_check_ok); + old_check_ok, + deleteTriggerOid, updateTriggerOid); /* Done -- clean up (but keep the lock) */ table_close(partRel, NoLock); @@ -9020,8 +9046,12 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, - bool old_check_ok, LOCKMODE lockmode) + bool old_check_ok, LOCKMODE lockmode, + Oid parentInsTrigger, Oid parentUpdTrigger) { + Oid insertTriggerOid, + updateTriggerOid; + AssertArg(OidIsValid(parentConstr)); if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) @@ -9035,14 +9065,16 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, * * If the relation is partitioned, drill down to do it to its partitions. */ + createForeignKeyCheckTriggers(RelationGetRelid(rel), + RelationGetRelid(pkrel), + fkconstraint, + parentConstr, + indexOid, + parentInsTrigger, parentUpdTrigger, + &insertTriggerOid, &updateTriggerOid); + if (rel->rd_rel->relkind == RELKIND_RELATION) { - createForeignKeyCheckTriggers(RelationGetRelid(rel), - RelationGetRelid(pkrel), - fkconstraint, - parentConstr, - indexOid); - /* * Tell Phase 3 to check that the constraint is satisfied by existing * rows. We can skip this during table creation, when requested @@ -9071,6 +9103,15 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { PartitionDesc pd = RelationGetPartitionDesc(rel); + Relation trigrel; + + /* + * Triggers of the foreign keys will be manipulated a bunch of times + * in the loop below. To avoid repeatedly opening/closing the trigger + * catalog relation, we open it here and pass it to the subroutines + * called below. + */ + trigrel = table_open(TriggerRelationId, RowExclusiveLock); /* * Recurse to take appropriate action on each partition; either we @@ -9112,7 +9153,10 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, numfks, mapped_fkattnum, pkattnum, - pfeqoperators)) + pfeqoperators, + insertTriggerOid, + updateTriggerOid, + trigrel)) { attached = true; break; @@ -9191,10 +9235,14 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, ppeqoperators, ffeqoperators, old_check_ok, - lockmode); + lockmode, + insertTriggerOid, + updateTriggerOid); table_close(partition, NoLock); } + + table_close(trigrel, RowExclusiveLock); } } @@ -9250,6 +9298,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) ScanKeyData key[2]; HeapTuple tuple; List *clone = NIL; + Relation trigrel; /* * Search for any constraints where this partition's parent is in the @@ -9279,6 +9328,14 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) systable_endscan(scan); table_close(pg_constraint, RowShareLock); + /* + * Triggers of the foreign keys will be manipulated a bunch of times + * in the loop below. To avoid repeatedly opening/closing the trigger + * catalog relation, we open it here and pass it to the subroutines + * called below. + */ + trigrel = table_open(TriggerRelationId, RowExclusiveLock); + attmap = build_attrmap_by_name(RelationGetDescr(partitionRel), RelationGetDescr(parentRel)); foreach(cell, clone) @@ -9296,6 +9353,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) Oid conppeqop[INDEX_MAX_KEYS]; Oid conffeqop[INDEX_MAX_KEYS]; Constraint *fkconstraint; + Oid deleteTriggerOid, + updateTriggerOid; tuple = SearchSysCache1(CONSTROID, constrOid); if (!HeapTupleIsValid(tuple)) @@ -9362,6 +9421,10 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) if (!OidIsValid(partIndexId)) elog(ERROR, "index for %u not found in partition %s", indexOid, RelationGetRelationName(partitionRel)); + + GetForeignKeyActionTriggers(constrForm, trigrel, + &deleteTriggerOid, &updateTriggerOid); + addFkRecurseReferenced(NULL, fkconstraint, fkRel, @@ -9374,11 +9437,15 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) conpfeqop, conppeqop, conffeqop, - true); + true, + deleteTriggerOid, + updateTriggerOid); table_close(fkRel, NoLock); ReleaseSysCache(tuple); } + + table_close(trigrel, RowExclusiveLock); } /* @@ -9401,6 +9468,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) List *partFKs; List *clone = NIL; ListCell *cell; + Relation trigrel; /* obtain a list of constraints that we need to clone */ foreach(cell, RelationGetFKeyList(parentRel)) @@ -9423,6 +9491,14 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) errmsg("foreign key constraints are not supported on foreign tables"))); /* + * Triggers of the foreign keys will be manipulated a bunch of times + * in the loop below. To avoid repeatedly opening/closing the trigger + * catalog relation, we open it here and pass it to the subroutines + * called below. + */ + trigrel = table_open(TriggerRelationId, RowExclusiveLock); + + /* * The constraint key may differ, if the columns in the partition are * different. This map is used to convert them. */ @@ -9451,6 +9527,8 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) ObjectAddress address, referenced; ListCell *cell; + Oid insertTriggerOid, + updateTriggerOid; tuple = SearchSysCache1(CONSTROID, parentConstrOid); if (!HeapTupleIsValid(tuple)) @@ -9479,6 +9557,10 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) for (int i = 0; i < numfks; i++) mapped_conkey[i] = attmap->attnums[conkey[i] - 1]; + GetForeignKeyCheckTriggers(trigrel, constrForm->oid, + constrForm->confrelid, constrForm->conrelid, + &insertTriggerOid, &updateTriggerOid); + /* * Before creating a new constraint, see whether any existing FKs are * fit for the purpose. If one is, attach the parent constraint to @@ -9497,7 +9579,10 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) numfks, mapped_conkey, confkey, - conpfeqop)) + conpfeqop, + insertTriggerOid, + updateTriggerOid, + trigrel)) { attached = true; table_close(pkrel, NoLock); @@ -9596,9 +9681,13 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) conppeqop, conffeqop, false, /* no old check exists */ - AccessExclusiveLock); + AccessExclusiveLock, + insertTriggerOid, + updateTriggerOid); table_close(pkrel, NoLock); } + + table_close(trigrel, RowExclusiveLock); } /* @@ -9619,13 +9708,15 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, int numfks, AttrNumber *mapped_conkey, AttrNumber *confkey, - Oid *conpfeqop) + Oid *conpfeqop, + Oid parentInsTrigger, + Oid parentUpdTrigger, + Relation trigrel) { HeapTuple parentConstrTup; Form_pg_constraint parentConstr; HeapTuple partcontup; Form_pg_constraint partConstr; - Relation trigrel; ScanKeyData key; SysScanDesc scan; HeapTuple trigtup; @@ -9689,12 +9780,10 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, * in the partition. We identify them because they have our constraint * OID, as well as being on the referenced rel. */ - trigrel = table_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) @@ -9727,13 +9816,135 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, } systable_endscan(scan); - table_close(trigrel, RowExclusiveLock); + + AttachForeignKeyCheckTriggers(fk, partRelid, parentInsTrigger, + parentUpdTrigger, trigrel); ConstraintSetParentConstraint(fk->conoid, parentConstrOid, partRelid); CommandCounterIncrement(); return true; } +static void +GetForeignKeyActionTriggers(Form_pg_constraint constrForm, + Relation trigrel, + Oid *deleteTriggerOid, + Oid *updateTriggerOid) +{ + ScanKeyData key; + SysScanDesc scan; + HeapTuple trigtup; + + *deleteTriggerOid = *updateTriggerOid = InvalidOid; + ScanKeyInit(&key, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(constrForm->oid)); + + 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 != constrForm->conrelid) + continue; + if (trgform->tgrelid != constrForm->confrelid) + continue; + if (TRIGGER_FOR_DELETE(trgform->tgtype)) + *deleteTriggerOid = trgform->oid; + else if (TRIGGER_FOR_UPDATE(trgform->tgtype)) + *updateTriggerOid = trgform->oid; + if (OidIsValid(*deleteTriggerOid) && OidIsValid(*updateTriggerOid)) + break; + } + + if (!OidIsValid(*deleteTriggerOid) || !OidIsValid(*updateTriggerOid)) + elog(ERROR, "could not find action triggers of foreign key constraint %u", + constrForm->oid); + + systable_endscan(scan); +} + +static void +GetForeignKeyCheckTriggers(Relation trigrel, + Oid conoid, Oid confrelid, Oid conrelid, + Oid *insertTriggerOid, + Oid *updateTriggerOid) +{ + ScanKeyData key; + SysScanDesc scan; + HeapTuple trigtup; + + *insertTriggerOid = *updateTriggerOid = InvalidOid; + ScanKeyInit(&key, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(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 != confrelid) + continue; + if (trgform->tgrelid != conrelid) + continue; + if (TRIGGER_FOR_INSERT(trgform->tgtype)) + *insertTriggerOid = trgform->oid; + else if (TRIGGER_FOR_UPDATE(trgform->tgtype)) + *updateTriggerOid = trgform->oid; + if (OidIsValid(*insertTriggerOid) && OidIsValid(*updateTriggerOid)) + break; + } + + if (!OidIsValid(*insertTriggerOid) || !OidIsValid(*updateTriggerOid)) + elog(ERROR, "could not find check triggers of foreign key constraint %u", + conoid); + + systable_endscan(scan); +} + +static void +AttachForeignKeyCheckTriggers(ForeignKeyCacheInfo *fk, Oid partRelid, + Oid parentInsTrigger, Oid parentUpdTrigger, + Relation trigrel) +{ + Oid insertTriggerOid, + updateTriggerOid; + + GetForeignKeyCheckTriggers(trigrel, + fk->conoid, fk->confrelid, fk->conrelid, + &insertTriggerOid, &updateTriggerOid); + + Assert(OidIsValid(insertTriggerOid) && OidIsValid(parentInsTrigger)); + TriggerSetParentTrigger(trigrel, insertTriggerOid, parentInsTrigger, + partRelid); + Assert(OidIsValid(updateTriggerOid) && OidIsValid(parentUpdTrigger)); + TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger, + partRelid); +} + +static void +DetachForeignKeyCheckTriggers(ForeignKeyCacheInfo *fk, Oid partRelid) +{ + Relation trigrel = table_open(TriggerRelationId, RowExclusiveLock); + Oid insertTriggerOid, + updateTriggerOid; + + GetForeignKeyCheckTriggers(trigrel, + fk->conoid, fk->confrelid, fk->conrelid, + &insertTriggerOid, &updateTriggerOid); + + Assert(OidIsValid(insertTriggerOid)); + TriggerSetParentTrigger(trigrel, insertTriggerOid, InvalidOid, partRelid); + Assert(OidIsValid(updateTriggerOid)); + TriggerSetParentTrigger(trigrel, updateTriggerOid, InvalidOid, partRelid); + + table_close(trigrel, RowExclusiveLock); +} /* * ALTER TABLE ALTER CONSTRAINT @@ -10534,10 +10745,12 @@ validateForeignKeyConstraint(char *conname, ExecDropSingleTupleTableSlot(slot); } -static void +static Oid CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, - Oid constraintOid, Oid indexOid, bool on_insert) + Oid constraintOid, Oid indexOid, Oid parentTrigOid, + bool on_insert) { + ObjectAddress trigAddress; CreateTrigStmt *fk_trigger; /* @@ -10577,11 +10790,14 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, fk_trigger->initdeferred = fkconstraint->initdeferred; fk_trigger->constrrel = NULL; - (void) CreateTrigger(fk_trigger, NULL, myRelOid, refRelOid, constraintOid, - indexOid, InvalidOid, InvalidOid, NULL, true, false); + trigAddress = CreateTrigger(fk_trigger, NULL, myRelOid, refRelOid, + constraintOid, indexOid, InvalidOid, + parentTrigOid, NULL, true, false); /* Make changes-so-far visible */ CommandCounterIncrement(); + + return trigAddress.objectId; } /* @@ -10591,9 +10807,12 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, */ static void createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint, - Oid constraintOid, Oid indexOid) + Oid constraintOid, Oid indexOid, + Oid parentDelTrigger, Oid parentUpdTrigger, + Oid *deleteTrigOid, Oid *updateTrigOid) { CreateTrigStmt *fk_trigger; + ObjectAddress trigAddress; /* * Build and execute a CREATE CONSTRAINT TRIGGER statement for the ON @@ -10645,9 +10864,12 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr break; } - (void) CreateTrigger(fk_trigger, NULL, refRelOid, RelationGetRelid(rel), - constraintOid, - indexOid, InvalidOid, InvalidOid, NULL, true, false); + trigAddress= CreateTrigger(fk_trigger, NULL, refRelOid, + RelationGetRelid(rel), + constraintOid, indexOid, InvalidOid, + parentDelTrigger, NULL, true, false); + if (deleteTrigOid) + *deleteTrigOid = trigAddress.objectId; /* Make changes-so-far visible */ CommandCounterIncrement(); @@ -10702,9 +10924,12 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr break; } - (void) CreateTrigger(fk_trigger, NULL, refRelOid, RelationGetRelid(rel), - constraintOid, - indexOid, InvalidOid, InvalidOid, NULL, true, false); + trigAddress = CreateTrigger(fk_trigger, NULL, refRelOid, + RelationGetRelid(rel), + constraintOid, indexOid, InvalidOid, + parentUpdTrigger, NULL, true, false); + if (updateTrigOid) + *updateTrigOid = trigAddress.objectId; } /* @@ -10715,12 +10940,16 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr static void createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, Oid constraintOid, - Oid indexOid) + Oid indexOid, + Oid parentInsTrigger, Oid parentUpdTrigger, + Oid *insertTrigOid, Oid *updateTrigOid) { - CreateFKCheckTrigger(myRelOid, refRelOid, fkconstraint, constraintOid, - indexOid, true); - CreateFKCheckTrigger(myRelOid, refRelOid, fkconstraint, constraintOid, - indexOid, false); + *insertTrigOid = CreateFKCheckTrigger(myRelOid, refRelOid, fkconstraint, + constraintOid, indexOid, + parentInsTrigger, true); + *updateTrigOid = CreateFKCheckTrigger(myRelOid, refRelOid, fkconstraint, + constraintOid, indexOid, + parentUpdTrigger, false); } /* @@ -16819,20 +17048,8 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) if (!TRIGGER_FOR_ROW(trigForm->tgtype)) continue; - /* - * Internal triggers require careful examination. Ideally, we don't - * clone them. However, if our parent is itself a partition, there - * might be internal triggers that must not be skipped; for example, - * triggers on our parent that are in turn clones from its parent (our - * grandparent) are marked internal, yet they are to be cloned. - * - * Note we dare not verify that the other trigger belongs to an - * ancestor relation of our parent, because that creates deadlock - * opportunities. - */ - if (trigForm->tgisinternal && - (!parent->rd_rel->relispartition || - !OidIsValid(trigForm->tgparentid))) + /* Don't clone internal triggers. They are dealt with separately. */ + if (trigForm->tgisinternal) continue; /* @@ -17081,7 +17298,11 @@ ATExecDetachPartition(Relation rel, RangeVar *name) createForeignKeyActionTriggers(partRel, conform->confrelid, fkconstraint, fk->conoid, - conform->conindid); + conform->conindid, + InvalidOid, InvalidOid, + NULL, NULL); + + DetachForeignKeyCheckTriggers(fk, RelationGetRelid(partRel)); ReleaseSysCache(contup); } @@ -17158,6 +17379,13 @@ DropClonedTriggersFromPartition(Oid partitionId) continue; /* + * Ignore internal triggers that are implementation objects of foreign + * keys. They will be dealt with separately. + */ + if (OidIsValid(pg_trigger->tgconstrrelid)) + continue; + + /* * This is ugly, but necessary: remove the dependency markings on the * trigger so that it can be removed. */ diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index c336b23..7b2d0de 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -136,8 +136,10 @@ static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); * given, stmt->funcname is ignored. * * parentTriggerOid, if nonzero, is a trigger that begets this one; so that - * if that trigger is dropped, this one should be too. (This is passed as - * Invalid by most callers; it's set here when recursing on a partition.) + * if that trigger is dropped, this one should be too. There are two cases + * when a nonzero value is passed for this: 1) when this function recurses to + * create the trigger on partitions, 2) when creating child foreign key + * triggers; see CreateFKCheckTrigger() and createForeignKeyActionTriggers(). * * If whenClause is passed, it is an already-transformed expression for * WHEN. In this case, we ignore any that may come in stmt->whenClause. @@ -188,6 +190,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, bool trigger_exists = false; Oid existing_constraint_oid = InvalidOid; bool existing_isInternal = false; + bool existing_isClone = false; if (OidIsValid(relOid)) rel = table_open(relOid, ShareRowExclusiveLock); @@ -726,6 +729,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, trigoid = oldtrigger->oid; existing_constraint_oid = oldtrigger->tgconstraint; existing_isInternal = oldtrigger->tgisinternal; + existing_isClone = OidIsValid(oldtrigger->tgparentid); trigger_exists = true; /* copy the tuple to use in CatalogTupleUpdate() */ tuple = heap_copytuple(tuple); @@ -752,17 +756,16 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, stmt->trigname, RelationGetRelationName(rel)))); /* - * An internal trigger cannot be replaced by a user-defined trigger. - * However, skip this test when in_partition, because then we're - * recursing from a partitioned table and the check was made at the - * parent level. Child triggers will always be marked "internal" (so - * this test does protect us from the user trying to replace a child - * trigger directly). + * An internal trigger or a child trigger (isClone) cannot be replaced + * by a user-defined trigger. However, skip this test when + * in_partition, because then we're recursing from a partitioned table + * and the check was made at the parent level. */ - if (existing_isInternal && !isInternal && !in_partition) + if ((existing_isInternal || existing_isClone) && !isInternal && + !in_partition) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("trigger \"%s\" for relation \"%s\" is an internal trigger", + errmsg("trigger \"%s\" for relation \"%s\" is an internal or a child trigger", stmt->trigname, RelationGetRelationName(rel)))); /* @@ -859,7 +862,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid); values[Anum_pg_trigger_tgtype - 1] = Int16GetDatum(tgtype); values[Anum_pg_trigger_tgenabled - 1] = CharGetDatum(TRIGGER_FIRES_ON_ORIGIN); - values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal || in_partition); + values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal); values[Anum_pg_trigger_tgconstrrelid - 1] = ObjectIdGetDatum(constrrelid); values[Anum_pg_trigger_tgconstrindid - 1] = ObjectIdGetDatum(indexOid); values[Anum_pg_trigger_tgconstraint - 1] = ObjectIdGetDatum(constraintOid); @@ -1228,6 +1231,82 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, return myself; } +/* + * TriggerSetParentTrigger + * Set a partition's trigger as child of its parent trigger, + * or remove the linkage if parentTrigId is InvalidOid. + * + * This updates the constraint's pg_trigger row to show it as inherited, and + * adds PARTITION dependencies to prevent the trigger from being deleted + * on its own. Alternatively, reverse that. + */ +void +TriggerSetParentTrigger(Relation trigRel, + Oid childTrigId, + Oid parentTrigId, + Oid childTableId) +{ + SysScanDesc tgscan; + ScanKeyData skey[1]; + Form_pg_trigger trigForm; + HeapTuple tuple, + newtup; + ObjectAddress depender; + ObjectAddress referenced; + + /* + * Find the trigger to delete. + */ + ScanKeyInit(&skey[0], + Anum_pg_trigger_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(childTrigId)); + + tgscan = systable_beginscan(trigRel, TriggerOidIndexId, true, + NULL, 1, skey); + + tuple = systable_getnext(tgscan); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "could not find tuple for trigger %u", childTrigId); + newtup = heap_copytuple(tuple); + trigForm = (Form_pg_trigger) GETSTRUCT(newtup); + if (OidIsValid(parentTrigId)) + { + /* don't allow setting parent for a constraint that already has one */ + Assert(trigForm->tgparentid == 0); + if (trigForm->tgparentid != InvalidOid) + elog(ERROR, "trigger %u already has a parent trigger", + childTrigId); + + trigForm->tgparentid = parentTrigId; + + CatalogTupleUpdate(trigRel, &tuple->t_self, newtup); + + ObjectAddressSet(depender, TriggerRelationId, childTrigId); + + ObjectAddressSet(referenced, TriggerRelationId, parentTrigId); + recordDependencyOn(&depender, &referenced, DEPENDENCY_PARTITION_PRI); + + ObjectAddressSet(referenced, RelationRelationId, childTableId); + recordDependencyOn(&depender, &referenced, DEPENDENCY_PARTITION_SEC); + } + else + { + trigForm->tgparentid = InvalidOid; + + CatalogTupleUpdate(trigRel, &tuple->t_self, newtup); + + deleteDependencyRecordsForClass(TriggerRelationId, childTrigId, + TriggerRelationId, + DEPENDENCY_PARTITION_PRI); + deleteDependencyRecordsForClass(TriggerRelationId, childTrigId, + RelationRelationId, + DEPENDENCY_PARTITION_SEC); + } + + systable_endscan(tgscan); +} + /* * Guts of trigger deletion. diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index dc1d41d..291405c 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -7905,7 +7905,25 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) tbinfo->dobj.name); resetPQExpBuffer(query); - if (fout->remoteVersion >= 90000) + if (fout->remoteVersion >= 130000) + { + /* + * NB: think not to use pretty=true in pg_get_triggerdef. It + * could result in non-forward-compatible dumps of WHEN clauses + * due to under-parenthesization. + */ + appendPQExpBuffer(query, + "SELECT tgname, " + "tgfoid::pg_catalog.regproc AS tgfname, " + "pg_catalog.pg_get_triggerdef(oid, false) AS tgdef, " + "tgenabled, tableoid, oid " + "FROM pg_catalog.pg_trigger t " + "WHERE tgrelid = '%u'::pg_catalog.oid " + "AND NOT tgisinternal " + "AND tgparentid = 0", + tbinfo->dobj.catId.oid); + } + else if (fout->remoteVersion >= 90000) { /* * NB: think not to use pretty=true in pg_get_triggerdef. It diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 07d6400..94d1afe 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2969,11 +2969,7 @@ describeOneTableDetails(const char *schemaname, " AND u.tgparentid = 0) AS parent" : "NULL AS parent"), oid); - if (pset.sversion >= 110000) - appendPQExpBufferStr(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D') \n" - " OR EXISTS (SELECT 1 FROM pg_catalog.pg_depend WHERE objid = t.oid \n" - " AND refclassid = 'pg_catalog.pg_trigger'::pg_catalog.regclass))"); - else if (pset.sversion >= 90000) + if (pset.sversion >= 90000) /* display/warn about disabled internal triggers */ appendPQExpBufferStr(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D'))"); else if (pset.sversion >= 80300) diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index e38d732..67cdb2d 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -154,7 +154,10 @@ extern ObjectAddress CreateTrigger(CreateTrigStmt *stmt, const char *queryString Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid, Oid funcoid, Oid parentTriggerOid, Node *whenClause, bool isInternal, bool in_partition); - +extern void TriggerSetParentTrigger(Relation trigRel, + Oid childTrigId, + Oid parentTrigId, + Oid childTableId); extern void RemoveTriggerById(Oid trigOid); extern Oid get_trigger_oid(Oid relid, const char *name, bool missing_ok); diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 213dff6..decb140 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2105,7 +2105,7 @@ select tgrelid::regclass::text, tgname, tgfoid::regproc, tgenabled, tgisinternal tgrelid | tgname | tgfoid | tgenabled | tgisinternal -----------+--------+-----------------+-----------+-------------- trigpart | trg1 | trigger_nothing | O | f - trigpart1 | trg1 | trigger_nothing | O | t + trigpart1 | trg1 | trigger_nothing | O | f (2 rows) create table trigpart3 (like trigpart); @@ -3216,7 +3216,7 @@ NOTICE: hello from funcA create or replace trigger my_trig after insert on parted_trig_1 for each row execute procedure funcB(); -- should fail -ERROR: trigger "my_trig" for relation "parted_trig_1" is an internal trigger +ERROR: trigger "my_trig" for relation "parted_trig_1" is an internal or a child trigger insert into parted_trig (a) values (50); NOTICE: hello from funcA drop trigger my_trig on parted_trig; -- 1.8.3.1