diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 3631b8a929..89bc865e28 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -483,7 +483,8 @@ static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstra int numfks, int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok); + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger); static void validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, int numfksetcols, const int16 *fksetcolsattnums, List *fksetcols); @@ -492,7 +493,8 @@ static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, int numfks, int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, - 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); @@ -500,15 +502,30 @@ 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(Relation trigrel, + Oid conoid, Oid confrelid, Oid conrelid, + Oid *deleteTriggerOid, + Oid *updateTriggerOid); +static void GetForeignKeyCheckTriggers(Relation trigrel, + Oid conoid, Oid confrelid, Oid conrelid, + Oid *insertTriggerOid, + Oid *updateTriggerOid); static void ATExecDropConstraint(Relation rel, const char *constrName, DropBehavior behavior, bool recurse, bool recursing, @@ -9367,7 +9384,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ffeqoperators, numfkdelsetcols, fkdelsetcols, - old_check_ok); + old_check_ok, + InvalidOid, InvalidOid); /* Now handle the referencing side. */ addFkRecurseReferencing(wqueue, fkconstraint, rel, pkrel, @@ -9382,7 +9400,8 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, numfkdelsetcols, fkdelsetcols, old_check_ok, - lockmode); + lockmode, + InvalidOid, InvalidOid); /* * Done. Close pk table, but keep lock until we've committed. @@ -9454,6 +9473,9 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, * 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, @@ -9462,7 +9484,8 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok) + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger) { ObjectAddress address; Oid constrOid; @@ -9470,6 +9493,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 @@ -9568,15 +9593,13 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, CommandCounterIncrement(); /* - * If the referenced table is a plain relation, create the action triggers - * that enforce the constraint. + * 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 @@ -9621,7 +9644,8 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, mapped_pkattnum, fkattnum, pfeqoperators, ppeqoperators, ffeqoperators, numfkdelsetcols, fkdelsetcols, - old_check_ok); + old_check_ok, + deleteTriggerOid, updateTriggerOid); /* Done -- clean up (but keep the lock) */ table_close(partRel, NoLock); @@ -9668,6 +9692,9 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, * old_check_ok signals that this constraint replaces an existing one that * was already validated (thus this one doesn't need validation). * lockmode is the lockmode to acquire on partitions when recursing. + * parentInsTrigger and parentUpdTrigger, when being recursively called on + * a partition, are the OIDs of the parent check triggers for INSERT and + * UPDATE respectively. */ static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, @@ -9675,8 +9702,12 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, int numfks, int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, - 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) @@ -9685,19 +9716,21 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, errmsg("foreign key constraints are not supported on foreign tables"))); /* - * If the referencing relation is a plain table, add the check triggers to - * it and, if necessary, schedule it to be checked in Phase 3. + * Add the check triggers to it and, if necessary, schedule it to be + * checked in Phase 3. * * 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 @@ -9726,6 +9759,15 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { PartitionDesc pd = RelationGetPartitionDesc(rel, true); + 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 @@ -9767,7 +9809,10 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, numfks, mapped_fkattnum, pkattnum, - pfeqoperators)) + pfeqoperators, + insertTriggerOid, + updateTriggerOid, + trigrel)) { attached = true; break; @@ -9850,10 +9895,14 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, numfkdelsetcols, fkdelsetcols, old_check_ok, - lockmode); + lockmode, + insertTriggerOid, + updateTriggerOid); table_close(partition, NoLock); } + + table_close(trigrel, RowExclusiveLock); } } @@ -9909,6 +9958,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 @@ -9938,6 +9988,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) @@ -9957,6 +10015,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) int numfkdelsetcols; AttrNumber confdelsetcols[INDEX_MAX_KEYS]; Constraint *fkconstraint; + Oid deleteTriggerOid, + updateTriggerOid; tuple = SearchSysCache1(CONSTROID, constrOid); if (!HeapTupleIsValid(tuple)) @@ -10025,6 +10085,16 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) if (!OidIsValid(partIndexId)) elog(ERROR, "index for %u not found in partition %s", indexOid, RelationGetRelationName(partitionRel)); + + /* + * Get the "action" triggers belonging to the constraint to pass as + * parent OIDs for similar triggers that will be created on the + * partition in addFkRecurseReferenced(). + */ + GetForeignKeyActionTriggers(trigrel, constrOid, + constrForm->confrelid, constrForm->conrelid, + &deleteTriggerOid, &updateTriggerOid); + addFkRecurseReferenced(NULL, fkconstraint, fkRel, @@ -10039,11 +10109,15 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) conffeqop, numfkdelsetcols, confdelsetcols, - true); + true, + deleteTriggerOid, + updateTriggerOid); table_close(fkRel, NoLock); ReleaseSysCache(tuple); } + + table_close(trigrel, RowExclusiveLock); } /* @@ -10066,6 +10140,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)) @@ -10087,6 +10162,14 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) (errcode(ERRCODE_WRONG_OBJECT_TYPE), 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. @@ -10118,6 +10201,8 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) ObjectAddress address, referenced; ListCell *cell; + Oid insertTriggerOid, + updateTriggerOid; tuple = SearchSysCache1(CONSTROID, parentConstrOid); if (!HeapTupleIsValid(tuple)) @@ -10147,6 +10232,19 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) for (int i = 0; i < numfks; i++) mapped_conkey[i] = attmap->attnums[conkey[i] - 1]; + /* + * Get the "check" triggers belonging to the constraint to pass as + * parent OIDs for similar triggers that will be created on the + * partition in addFkRecurseReferencing(). They are also passed to + * tryAttachPartitionForeignKey() below to simply assign as parents to + * the partition's existing "check" triggers, that is, if the + * corresponding constraints is deemed attachable to the parent + * constraint. + */ + 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 @@ -10165,7 +10263,10 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) numfks, mapped_conkey, confkey, - conpfeqop)) + conpfeqop, + insertTriggerOid, + updateTriggerOid, + trigrel)) { attached = true; table_close(pkrel, NoLock); @@ -10268,9 +10369,13 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) numfkdelsetcols, confdelsetcols, false, /* no old check exists */ - AccessExclusiveLock); + AccessExclusiveLock, + insertTriggerOid, + updateTriggerOid); table_close(pkrel, NoLock); } + + table_close(trigrel, RowExclusiveLock); } /* @@ -10291,16 +10396,20 @@ 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; + Oid insertTriggerOid, + updateTriggerOid; parentConstrTup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(parentConstrOid)); @@ -10361,12 +10470,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) @@ -10399,13 +10506,136 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, } systable_endscan(scan); - table_close(trigrel, RowExclusiveLock); ConstraintSetParentConstraint(fk->conoid, parentConstrOid, partRelid); + + /* + * Like the constraint, attach partition's "check" triggers to the + * corresponding parent triggers. + */ + 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); + CommandCounterIncrement(); return true; } +/* + * GetForeignKeyActionTriggers + * Returns delete and update "action" triggers of the given relation + * belonging to the given constraint + */ +static void +GetForeignKeyActionTriggers(Relation trigrel, + Oid conoid, Oid confrelid, Oid conrelid, + Oid *deleteTriggerOid, + Oid *updateTriggerOid) +{ + ScanKeyData key; + SysScanDesc scan; + HeapTuple trigtup; + + *deleteTriggerOid = *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 != conrelid) + continue; + if (trgform->tgrelid != confrelid) + continue; + if (TRIGGER_FOR_DELETE(trgform->tgtype)) + { + Assert(*deleteTriggerOid == InvalidOid); + *deleteTriggerOid = trgform->oid; + } + else if (TRIGGER_FOR_UPDATE(trgform->tgtype)) + { + Assert(*updateTriggerOid == InvalidOid); + *updateTriggerOid = trgform->oid; + } + if (OidIsValid(*deleteTriggerOid) && OidIsValid(*updateTriggerOid)) + break; + } + + if (!OidIsValid(*deleteTriggerOid)) + elog(ERROR, "could not find ON DELETE action trigger of foreign key constraint %u", + conoid); + if (!OidIsValid(*updateTriggerOid)) + elog(ERROR, "could not find ON UPDATE action trigger of foreign key constraint %u", + conoid); + + systable_endscan(scan); +} + +/* + * GetForeignKeyCheckTriggers + * Returns insert and update "check" triggers of the given relation + * belonging to the given constraint + */ +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)) + { + Assert(*insertTriggerOid == InvalidOid); + *insertTriggerOid = trgform->oid; + } + else if (TRIGGER_FOR_UPDATE(trgform->tgtype)) + { + Assert(*updateTriggerOid == InvalidOid); + *updateTriggerOid = trgform->oid; + } + if (OidIsValid(*insertTriggerOid) && OidIsValid(*updateTriggerOid)) + break; + } + + if (!OidIsValid(*insertTriggerOid)) + elog(ERROR, "could not find ON INSERT check triggers of foreign key constraint %u", + conoid); + if (!OidIsValid(*updateTriggerOid)) + elog(ERROR, "could not find ON UPDATE check triggers of foreign key constraint %u", + conoid); + + systable_endscan(scan); +} /* * ALTER TABLE ALTER CONSTRAINT @@ -11323,10 +11553,19 @@ validateForeignKeyConstraint(char *conname, ExecDropSingleTupleTableSlot(slot); } -static void +/* + * CreateFKCheckTrigger + * Creates the insert (on_insert=true) or update "check" trigger that + * implements a given foreign key + * + * Returns the OID of the so created trigger. + */ +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; /* @@ -11366,23 +11605,32 @@ 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; } /* * createForeignKeyActionTriggers * Create the referenced-side "action" triggers that implement a foreign * key. + * + * Returns the OIDs of the so created triggers in *deleteTrigOid and + * *updateTrigOid. */ 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 @@ -11434,9 +11682,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(); @@ -11491,25 +11742,35 @@ 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; } /* * createForeignKeyCheckTriggers * Create the referencing-side "check" triggers that implement a foreign * key. + * + * Returns the OIDs of the so created triggers in *insertTrigOid and + * *updateTrigOid. */ 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); } /* @@ -17859,19 +18120,10 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) 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. + * Don't clone internal triggers, because the constraint cloning code + * will. */ - if (trigForm->tgisinternal && - (!parent->rd_rel->relispartition || - !OidIsValid(trigForm->tgparentid))) + if (trigForm->tgisinternal) continue; /* @@ -18173,6 +18425,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, new_repl[Natts_pg_class]; HeapTuple tuple, newtuple; + Relation trigrel = NULL; if (concurrent) { @@ -18191,12 +18444,16 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, * additional action triggers. */ fks = copyObject(RelationGetFKeyList(partRel)); + if (fks != NIL) + trigrel = table_open(TriggerRelationId, RowExclusiveLock); foreach(cell, fks) { ForeignKeyCacheInfo *fk = lfirst(cell); HeapTuple contup; Form_pg_constraint conform; Constraint *fkconstraint; + Oid insertTriggerOid, + updateTriggerOid; contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid)); if (!HeapTupleIsValid(contup)) @@ -18214,6 +18471,20 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, /* unset conparentid and adjust conislocal, coninhcount, etc. */ ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid); + /* + * Also, look up the partition's "check" triggers corresponding to the + * constraint being detached and detach them from the parent triggers. + */ + GetForeignKeyCheckTriggers(trigrel, + fk->conoid, fk->confrelid, fk->conrelid, + &insertTriggerOid, &updateTriggerOid); + Assert(OidIsValid(insertTriggerOid)); + TriggerSetParentTrigger(trigrel, insertTriggerOid, InvalidOid, + RelationGetRelid(partRel)); + Assert(OidIsValid(updateTriggerOid)); + TriggerSetParentTrigger(trigrel, updateTriggerOid, InvalidOid, + RelationGetRelid(partRel)); + /* * Make the action triggers on the referenced relation. When this was * a partition the action triggers pointed to the parent rel (they @@ -18228,11 +18499,15 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, createForeignKeyActionTriggers(partRel, conform->confrelid, fkconstraint, fk->conoid, - conform->conindid); + conform->conindid, + InvalidOid, InvalidOid, + NULL, NULL); ReleaseSysCache(contup); } list_free_deep(fks); + if (trigrel) + table_close(trigrel, RowExclusiveLock); /* * Any sub-constraints that are in the referenced-side of a larger @@ -18457,6 +18732,14 @@ DropClonedTriggersFromPartition(Oid partitionId) if (!OidIsValid(pg_trigger->tgparentid)) continue; + /* + * Ignore internal triggers that are implementation objects of foreign + * keys, because these will be detached when the foreign keys + * themselves are. + */ + 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 7c8826089b..452b743f21 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -132,8 +132,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. @@ -202,6 +204,7 @@ CreateTriggerFiringOn(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); @@ -741,6 +744,7 @@ CreateTriggerFiringOn(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); @@ -767,17 +771,16 @@ CreateTriggerFiringOn(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)))); /* @@ -876,7 +879,7 @@ CreateTriggerFiringOn(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] = trigger_fires_when; - 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); @@ -1245,6 +1248,82 @@ CreateTriggerFiringOn(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 */ + if (OidIsValid(trigForm->tgparentid)) + 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); + } + + heap_freetuple(newtup); + 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 4fec88b5e5..59cd02ebb1 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -7195,7 +7195,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) i_tgconstrrelid, i_tgconstrrelname, i_tgenabled, - i_tgisinternal, + i_tgispartition, i_tgdeferrable, i_tginitdeferred, i_tgdef; @@ -7224,7 +7224,31 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) } appendPQExpBufferChar(tbloids, '}'); - if (fout->remoteVersion >= 130000) + if (fout->remoteVersion >= 150000) + { + /* + * 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. + * + * NB: We need to see partition triggers in case the tgenabled flag + * has been changed from the parent. + */ + appendPQExpBuffer(query, + "SELECT t.tgrelid, t.tgname, " + "t.tgfoid::pg_catalog.regproc AS tgfname, " + "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, " + "t.tgenabled, t.tableoid, t.oid, " + "t.tgparentid <> 0 AS tgispartition\n" + "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" + "JOIN pg_catalog.pg_trigger t ON (src.tbloid = t.tgrelid) " + "LEFT JOIN pg_catalog.pg_trigger u ON (u.oid = t.tgparentid) " + "WHERE ((NOT t.tgisinternal AND t.tgparentid = 0) " + "OR t.tgenabled != u.tgenabled) " + "ORDER BY t.tgrelid, t.tgname", + tbloids->data); + } + else if (fout->remoteVersion >= 130000) { /* * NB: think not to use pretty=true in pg_get_triggerdef. It could @@ -7238,7 +7262,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) "SELECT t.tgrelid, t.tgname, " "t.tgfoid::pg_catalog.regproc AS tgfname, " "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, " - "t.tgenabled, t.tableoid, t.oid, t.tgisinternal\n" + "t.tgenabled, t.tableoid, t.oid, t.tgisinternal as tgispartition\n" "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_trigger t ON (src.tbloid = t.tgrelid) " "LEFT JOIN pg_catalog.pg_trigger u ON (u.oid = t.tgparentid) " @@ -7259,7 +7283,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) "SELECT t.tgrelid, t.tgname, " "t.tgfoid::pg_catalog.regproc AS tgfname, " "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, " - "t.tgenabled, t.tableoid, t.oid, t.tgisinternal " + "t.tgenabled, t.tableoid, t.oid, t.tgisinternal as tgispartition " "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_trigger t ON (src.tbloid = t.tgrelid) " "LEFT JOIN pg_catalog.pg_depend AS d ON " @@ -7278,7 +7302,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) "SELECT t.tgrelid, t.tgname, " "t.tgfoid::pg_catalog.regproc AS tgfname, " "pg_catalog.pg_get_triggerdef(t.oid, false) AS tgdef, " - "t.tgenabled, false as tgisinternal, " + "t.tgenabled, false as tgispartition, " "t.tableoid, t.oid " "FROM unnest('%s'::pg_catalog.oid[]) AS src(tbloid)\n" "JOIN pg_catalog.pg_trigger t ON (src.tbloid = t.tgrelid) " @@ -7304,7 +7328,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) i_tgconstrrelid = PQfnumber(res, "tgconstrrelid"); i_tgconstrrelname = PQfnumber(res, "tgconstrrelname"); i_tgenabled = PQfnumber(res, "tgenabled"); - i_tgisinternal = PQfnumber(res, "tgisinternal"); + i_tgispartition = PQfnumber(res, "tgispartition"); i_tgdeferrable = PQfnumber(res, "tgdeferrable"); i_tginitdeferred = PQfnumber(res, "tginitdeferred"); i_tgdef = PQfnumber(res, "tgdef"); @@ -7354,7 +7378,7 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables) tginfo[j].dobj.namespace = tbinfo->dobj.namespace; tginfo[j].tgtable = tbinfo; tginfo[j].tgenabled = *(PQgetvalue(res, j, i_tgenabled)); - tginfo[j].tgisinternal = *(PQgetvalue(res, j, i_tgisinternal)) == 't'; + tginfo[j].tgispartition = *(PQgetvalue(res, j, i_tgispartition)) == 't'; if (i_tgdef >= 0) { tginfo[j].tgdef = pg_strdup(PQgetvalue(res, j, i_tgdef)); @@ -16771,11 +16795,13 @@ dumpTrigger(Archive *fout, const TriggerInfo *tginfo) "pg_catalog.pg_trigger", "TRIGGER", trigidentity->data); - if (tginfo->tgisinternal) + if (tginfo->tgispartition) { + Assert(tbinfo->ispartition); + /* - * Triggers marked internal only appear here because their 'tgenabled' - * flag differs from its parent's. The trigger is created already, so + * Partition triggers only appear here because their 'tgenabled' flag + * differs from its parent's. The trigger is created already, so * remove the CREATE and replace it with an ALTER. (Clear out the * DROP query too, so that pg_dump --create does not cause errors.) */ diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index f9deb321ac..c8799f0565 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -443,7 +443,7 @@ typedef struct _triggerInfo Oid tgconstrrelid; char *tgconstrrelname; char tgenabled; - bool tgisinternal; + bool tgispartition; bool tgdeferrable; bool tginitdeferred; char *tgdef; diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index c28788e84f..0615de5325 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -3001,7 +3001,15 @@ describeOneTableDetails(const char *schemaname, " AND u.tgparentid = 0) AS parent" : "NULL AS parent"), oid); - if (pset.sversion >= 110000) + + /* + * tgisinternal is set true for inherited triggers of partitions in + * servers between v11 and v14, though these must still be shown to + * the user. So we use another property that is true for such + * inherited triggers to avoid them being hidden, which is their + * dependendence on another trigger. + */ + if (pset.sversion >= 110000 && pset.sversion < 150000) 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))"); diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 9ef7f6d768..489c93de92 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -160,6 +160,10 @@ extern ObjectAddress CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *que Node *whenClause, bool isInternal, bool in_partition, char trigger_fires_when); +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 5d124cf96f..5c0e7c2b79 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); @@ -3311,7 +3311,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;