I found some unused and some redundant code in ri_triggers.c that was
left around by some previous changes that aimed to optimize away certain
trigger invocations. See attached patches.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 1fe8dac15a0013ef348dce1586db5af7bb02639a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 19 Jul 2018 08:20:59 +0200
Subject: [PATCH 1/2] Remove dead foreign key optimization code
The ri_KeysEqual() calls in the foreign-key trigger functions to
optimize away some updates are useless because since
adfeef55cbcc5dc72a772777f88c1be05a70dfee those triggers are not enqueued
at all. (It's also not useful to keep these checks as some kind of
backstop, since it's also semantically correct to just run the full
check even with equal keys.)
---
src/backend/utils/adt/ri_triggers.c | 51 -----------------------------
1 file changed, 51 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c
b/src/backend/utils/adt/ri_triggers.c
index fc034ce601..b674011aa7 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -744,20 +744,6 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
break;
}
- /*
- * In UPDATE, no need to do anything if old and new
keys are equal
- */
- if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
- {
- HeapTuple new_row = trigdata->tg_newtuple;
-
- if (ri_KeysEqual(pk_rel, old_row, new_row,
riinfo, true))
- {
- heap_close(fk_rel, RowShareLock);
- return PointerGetDatum(NULL);
- }
- }
-
/*
* If another PK row now exists providing the old key
values, we
* should not do anything. However, this check should
only be
@@ -1098,15 +1084,6 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
break;
}
- /*
- * No need to do anything if old and new keys are equal
- */
- if (ri_KeysEqual(pk_rel, old_row, new_row, riinfo,
true))
- {
- heap_close(fk_rel, RowExclusiveLock);
- return PointerGetDatum(NULL);
- }
-
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
@@ -1316,20 +1293,6 @@ ri_setnull(TriggerData *trigdata)
break;
}
- /*
- * In UPDATE, no need to do anything if old and new
keys are equal
- */
- if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
- {
- HeapTuple new_row = trigdata->tg_newtuple;
-
- if (ri_KeysEqual(pk_rel, old_row, new_row,
riinfo, true))
- {
- heap_close(fk_rel, RowExclusiveLock);
- return PointerGetDatum(NULL);
- }
- }
-
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
@@ -1536,20 +1499,6 @@ ri_setdefault(TriggerData *trigdata)
break;
}
- /*
- * In UPDATE, no need to do anything if old and new
keys are equal
- */
- if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
- {
- HeapTuple new_row = trigdata->tg_newtuple;
-
- if (ri_KeysEqual(pk_rel, old_row, new_row,
riinfo, true))
- {
- heap_close(fk_rel, RowExclusiveLock);
- return PointerGetDatum(NULL);
- }
- }
-
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
--
2.18.0
From d2bcff7d75bfbe07bcb27100fef8b3bedf279d2a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 19 Jul 2018 08:37:32 +0200
Subject: [PATCH 2/2] Apply RI trigger skipping tests also for DELETE
The tests added in cfa0f4255bb0f5550d37a01c4d8fe2966d20040c to skip
firing an RI trigger if any old key value is NULL can also be applied
for DELETE. This should give a performance gain in those cases, and it
also saves a lot of duplicate code in the actual RI triggers. (That
code was already dead code for the UPDATE cases.)
---
src/backend/commands/trigger.c | 4 +-
src/backend/utils/adt/ri_triggers.c | 105 ++--------------------------
2 files changed, 6 insertions(+), 103 deletions(-)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 2436692eb8..f1927bf91a 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -5946,12 +5946,12 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo
*relinfo,
* 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))
+ if (TRIGGER_FIRED_BY_UPDATE(event) ||
TRIGGER_FIRED_BY_DELETE(event))
{
switch (RI_FKey_trigger_type(trigger->tgfoid))
{
case RI_TRIGGER_PK:
- /* Update on trigger's PK table */
+ /* Update or delete on trigger's PK
table */
if
(!RI_FKey_pk_upd_check_required(trigger, rel,
oldtup, newtup))
{
diff --git a/src/backend/utils/adt/ri_triggers.c
b/src/backend/utils/adt/ri_triggers.c
index b674011aa7..b71210da27 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -724,25 +724,6 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
*/
case FKCONSTR_MATCH_SIMPLE:
case FKCONSTR_MATCH_FULL:
- switch (ri_NullCheck(RelationGetDescr(pk_rel), old_row,
riinfo, true))
- {
- case RI_KEYS_ALL_NULL:
- case RI_KEYS_SOME_NULL:
-
- /*
- * No check needed - there cannot be
any reference to old
- * key if it contains a NULL
- */
- heap_close(fk_rel, RowShareLock);
- return PointerGetDatum(NULL);
-
- case RI_KEYS_NONE_NULL:
-
- /*
- * Have a full qualified key - continue
below
- */
- break;
- }
/*
* If another PK row now exists providing the old key
values, we
@@ -900,26 +881,6 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
*/
case FKCONSTR_MATCH_SIMPLE:
case FKCONSTR_MATCH_FULL:
- switch (ri_NullCheck(RelationGetDescr(pk_rel), old_row,
riinfo, true))
- {
- case RI_KEYS_ALL_NULL:
- case RI_KEYS_SOME_NULL:
-
- /*
- * No check needed - there cannot be
any reference to old
- * key if it contains a NULL
- */
- heap_close(fk_rel, RowExclusiveLock);
- return PointerGetDatum(NULL);
-
- case RI_KEYS_NONE_NULL:
-
- /*
- * Have a full qualified key - continue
below
- */
- break;
- }
-
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
@@ -1064,26 +1025,6 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
*/
case FKCONSTR_MATCH_SIMPLE:
case FKCONSTR_MATCH_FULL:
- switch (ri_NullCheck(RelationGetDescr(pk_rel), old_row,
riinfo, true))
- {
- case RI_KEYS_ALL_NULL:
- case RI_KEYS_SOME_NULL:
-
- /*
- * No check needed - there cannot be
any reference to old
- * key if it contains a NULL
- */
- heap_close(fk_rel, RowExclusiveLock);
- return PointerGetDatum(NULL);
-
- case RI_KEYS_NONE_NULL:
-
- /*
- * Have a full qualified key - continue
below
- */
- break;
- }
-
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
@@ -1273,26 +1214,6 @@ ri_setnull(TriggerData *trigdata)
*/
case FKCONSTR_MATCH_SIMPLE:
case FKCONSTR_MATCH_FULL:
- switch (ri_NullCheck(RelationGetDescr(pk_rel), old_row,
riinfo, true))
- {
- case RI_KEYS_ALL_NULL:
- case RI_KEYS_SOME_NULL:
-
- /*
- * No check needed - there cannot be
any reference to old
- * key if it contains a NULL
- */
- heap_close(fk_rel, RowExclusiveLock);
- return PointerGetDatum(NULL);
-
- case RI_KEYS_NONE_NULL:
-
- /*
- * Have a full qualified key - continue
below
- */
- break;
- }
-
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
@@ -1479,26 +1400,6 @@ ri_setdefault(TriggerData *trigdata)
*/
case FKCONSTR_MATCH_SIMPLE:
case FKCONSTR_MATCH_FULL:
- switch (ri_NullCheck(RelationGetDescr(pk_rel), old_row,
riinfo, true))
- {
- case RI_KEYS_ALL_NULL:
- case RI_KEYS_SOME_NULL:
-
- /*
- * No check needed - there cannot be
any reference to old
- * key if it contains a NULL
- */
- heap_close(fk_rel, RowExclusiveLock);
- return PointerGetDatum(NULL);
-
- case RI_KEYS_NONE_NULL:
-
- /*
- * Have a full qualified key - continue
below
- */
- break;
- }
-
if (SPI_connect() != SPI_OK_CONNECT)
elog(ERROR, "SPI_connect failed");
@@ -1617,11 +1518,13 @@ ri_setdefault(TriggerData *trigdata)
/* ----------
* RI_FKey_pk_upd_check_required -
*
- * Check if we really need to fire the RI trigger for an update to a PK
+ * Check if we really need to fire the RI trigger for an update or delete
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.
+ *
+ * new_row will be NULL if this is called for a delete.
* ----------
*/
bool
@@ -1648,7 +1551,7 @@ RI_FKey_pk_upd_check_required(Trigger *trigger, Relation
pk_rel,
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))
+ if (new_row && ri_KeysEqual(pk_rel, old_row, new_row,
riinfo, true))
return false;
/* Else we need to fire the trigger. */
--
2.18.0
