diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 57fcfbeefd94baf9e06b9d6764d46f3869cc7976..834b694ec6b872081855f009a3919ea2ab5c1b8a 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -4582,39 +4582,30 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, continue; /* - * If this is an UPDATE of a PK table or FK table that does not change - * the PK or FK respectively, we can skip queuing the event: there is - * no need to fire the trigger. + * If the trigger is a foreign key enforcement trigger, there are + * certain cases where we can skip queueing the event because we can + * tell by inspection that the FK constraint will still pass. */ if (TRIGGER_FIRED_BY_UPDATE(event)) { switch (RI_FKey_trigger_type(trigger->tgfoid)) { case RI_TRIGGER_PK: - /* Update on PK table */ - if (RI_FKey_keyequal_upd_pk(trigger, rel, oldtup, newtup)) + /* Update on trigger's PK table */ + if (!RI_FKey_pk_upd_check_required(trigger, rel, + oldtup, newtup)) { - /* key unchanged, so skip queuing this event */ + /* skip queuing this event */ continue; } break; case RI_TRIGGER_FK: - - /* - * Update on FK table - * - * There is one exception when updating FK tables: if the - * updated row was inserted by our own transaction and the - * FK is deferred, we still need to fire the trigger. This - * is because our UPDATE will invalidate the INSERT so the - * end-of-transaction INSERT RI trigger will not do - * anything, so we have to do the check for the UPDATE - * anyway. - */ - if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(oldtup->t_data)) && - RI_FKey_keyequal_upd_fk(trigger, rel, oldtup, newtup)) + /* Update on trigger's FK table */ + if (!RI_FKey_fk_upd_check_required(trigger, rel, + oldtup, newtup)) { + /* skip queuing this event */ continue; } break; diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index f83f20be7abd56d89df41409ca4ffc425f0dd922..d61d1e0b3658b25cf7cebe740f23e1c64b44606d 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -1928,7 +1928,7 @@ RI_FKey_setdefault_del(PG_FUNCTION_ARGS) * If we just deleted the PK row whose key was equal to the FK * columns' default values, and a referencing row exists in the FK * table, we would have updated that row to the same values it - * already had --- and RI_FKey_keyequal_upd_fk would therefore + * already had --- and RI_FKey_fk_upd_check_required would hence * believe no check is necessary. So we need to do another lookup * now and in case a reference still exists, abort the operation. * That is already implemented in the NO ACTION trigger, so just @@ -2125,7 +2125,7 @@ RI_FKey_setdefault_upd(PG_FUNCTION_ARGS) * If we just updated the PK row whose key was equal to the FK * columns' default values, and a referencing row exists in the FK * table, we would have updated that row to the same values it - * already had --- and RI_FKey_keyequal_upd_fk would therefore + * already had --- and RI_FKey_fk_upd_check_required would hence * believe no check is necessary. So we need to do another lookup * now and in case a reference still exists, abort the operation. * That is already implemented in the NO ACTION trigger, so just @@ -2158,16 +2158,18 @@ RI_FKey_setdefault_upd(PG_FUNCTION_ARGS) /* ---------- - * RI_FKey_keyequal_upd_pk - + * RI_FKey_pk_upd_check_required - * - * Check if we have a key change on an update to a PK relation. This is - * used by the AFTER trigger queue manager to see if it can skip queuing - * an instance of an RI trigger. + * Check if we really need to fire the RI trigger for an update to a PK + * relation. This is called by the AFTER trigger queue manager to see if + * it can skip queuing an instance of an RI trigger. Returns TRUE if the + * trigger must be fired, FALSE if we can prove the constraint will still + * be satisfied. * ---------- */ bool -RI_FKey_keyequal_upd_pk(Trigger *trigger, Relation pk_rel, - HeapTuple old_row, HeapTuple new_row) +RI_FKey_pk_upd_check_required(Trigger *trigger, Relation pk_rel, + HeapTuple old_row, HeapTuple new_row) { RI_ConstraintInfo riinfo; @@ -2177,19 +2179,32 @@ RI_FKey_keyequal_upd_pk(Trigger *trigger, Relation pk_rel, ri_FetchConstraintInfo(&riinfo, trigger, pk_rel, true); /* - * Nothing to do if no column names to compare given + * Nothing to do if no columns (satisfaction of such a constraint only + * requires existence of a PK row, and this update won't change that). */ if (riinfo.nkeys == 0) - return true; + return false; switch (riinfo.confmatchtype) { case FKCONSTR_MATCH_SIMPLE: case FKCONSTR_MATCH_FULL: - /* Return true if keys are equal */ - return ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true); - /* Handle MATCH PARTIAL set null delete. */ + /* + * If any old key value is NULL, the row could not have been + * referenced by an FK row, so no check is needed. + */ + if (ri_NullCheck(old_row, &riinfo, true) != RI_KEYS_NONE_NULL) + return false; + + /* If all old and new key values are equal, no check is needed */ + if (ri_KeysEqual(pk_rel, old_row, new_row, &riinfo, true)) + return false; + + /* Else we need to fire the trigger. */ + return true; + + /* Handle MATCH PARTIAL check. */ case FKCONSTR_MATCH_PARTIAL: ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -2207,16 +2222,18 @@ RI_FKey_keyequal_upd_pk(Trigger *trigger, Relation pk_rel, } /* ---------- - * RI_FKey_keyequal_upd_fk - + * RI_FKey_fk_upd_check_required - * - * Check if we have a key change on an update to an FK relation. This is - * used by the AFTER trigger queue manager to see if it can skip queuing - * an instance of an RI trigger. + * Check if we really need to fire the RI trigger for an update to an FK + * relation. This is called by the AFTER trigger queue manager to see if + * it can skip queuing an instance of an RI trigger. Returns TRUE if the + * trigger must be fired, FALSE if we can prove the constraint will still + * be satisfied. * ---------- */ bool -RI_FKey_keyequal_upd_fk(Trigger *trigger, Relation fk_rel, - HeapTuple old_row, HeapTuple new_row) +RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel, + HeapTuple old_row, HeapTuple new_row) { RI_ConstraintInfo riinfo; @@ -2226,19 +2243,78 @@ RI_FKey_keyequal_upd_fk(Trigger *trigger, Relation fk_rel, ri_FetchConstraintInfo(&riinfo, trigger, fk_rel, false); /* - * Nothing to do if no column names to compare given + * Nothing to do if no columns (satisfaction of such a constraint only + * requires existence of a PK row, and this update won't change that). */ if (riinfo.nkeys == 0) - return true; + return false; switch (riinfo.confmatchtype) { case FKCONSTR_MATCH_SIMPLE: + /* + * If any new key value is NULL, the row must satisfy the + * constraint, so no check is needed. + */ + if (ri_NullCheck(new_row, &riinfo, false) != RI_KEYS_NONE_NULL) + return false; + + /* + * If the original row was inserted by our own transaction, we + * must fire the trigger whether or not the keys are equal. This + * is because our UPDATE will invalidate the INSERT so that the + * INSERT RI trigger will not do anything; so we had better do the + * UPDATE check. (We could skip this if we knew the INSERT + * trigger already fired, but there is no easy way to know that.) + */ + if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data))) + return true; + + /* If all old and new key values are equal, no check is needed */ + if (ri_KeysEqual(fk_rel, old_row, new_row, &riinfo, false)) + return false; + + /* Else we need to fire the trigger. */ + return true; + case FKCONSTR_MATCH_FULL: - /* Return true if keys are equal */ - return ri_KeysEqual(fk_rel, old_row, new_row, &riinfo, false); + /* + * If all new key values are NULL, the row must satisfy the + * constraint, so no check is needed. On the other hand, if only + * some of them are NULL, the row must fail the constraint. We + * must not throw error here, because the row might get + * invalidated before the constraint is to be checked, but we + * should queue the event to apply the check later. + */ + switch (ri_NullCheck(new_row, &riinfo, false)) + { + case RI_KEYS_ALL_NULL: + return false; + case RI_KEYS_SOME_NULL: + return true; + case RI_KEYS_NONE_NULL: + break; /* continue with the check */ + } + + /* + * If the original row was inserted by our own transaction, we + * must fire the trigger whether or not the keys are equal. This + * is because our UPDATE will invalidate the INSERT so that the + * INSERT RI trigger will not do anything; so we had better do the + * UPDATE check. (We could skip this if we knew the INSERT + * trigger already fired, but there is no easy way to know that.) + */ + if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data))) + return true; - /* Handle MATCH PARTIAL set null delete. */ + /* If all old and new key values are equal, no check is needed */ + if (ri_KeysEqual(fk_rel, old_row, new_row, &riinfo, false)) + return false; + + /* Else we need to fire the trigger. */ + return true; + + /* Handle MATCH PARTIAL check. */ case FKCONSTR_MATCH_PARTIAL: ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -3376,6 +3452,11 @@ ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan) * ri_KeysEqual - * * Check if all key values in OLD and NEW are equal. + * + * Note: at some point we might wish to redefine this as checking for + * "IS NOT DISTINCT" rather than "=", that is, allow two nulls to be + * considered equal. Currently there is no need since all callers have + * previously found at least one of the rows to contain no nulls. * ---------- */ static bool diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 93033414d914131ef3357f23cb8aecd7ed643b30..e790b9d3009bf9e3eb11f8cc8643b8860669150c 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -191,10 +191,10 @@ extern bool AfterTriggerPendingOnRel(Oid relid); /* * in utils/adt/ri_triggers.c */ -extern bool RI_FKey_keyequal_upd_pk(Trigger *trigger, Relation pk_rel, - HeapTuple old_row, HeapTuple new_row); -extern bool RI_FKey_keyequal_upd_fk(Trigger *trigger, Relation fk_rel, - HeapTuple old_row, HeapTuple new_row); +extern bool RI_FKey_pk_upd_check_required(Trigger *trigger, Relation pk_rel, + HeapTuple old_row, HeapTuple new_row); +extern bool RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel, + HeapTuple old_row, HeapTuple new_row); extern bool RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel);