Skip to content

Commit adfeef5

Browse files
author
Neil Conway
committed
When enqueueing after-row triggers for updates of a table with a foreign
key, compare the new and old row versions. If the foreign key column has not changed, we needn't enqueue the trigger, since the update cannot violate the foreign key. This optimization was previously applied in the RI trigger function, but it is more efficient to avoid firing the trigger altogether. Per recent discussion on pgsql-hackers. Also add a regression test for some unintuitive foreign key behavior, and refactor some code that deals with the OIDs of the various RI trigger functions.
1 parent f99b75b commit adfeef5

File tree

6 files changed

+256
-150
lines changed

6 files changed

+256
-150
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.158 2005/05/30 06:52:38 neilc Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.159 2005/05/30 07:20:58 neilc Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -156,12 +156,6 @@ typedef struct NewColumnValue
156156
} NewColumnValue;
157157

158158

159-
/* Used by attribute and relation renaming routines: */
160-
#define RI_TRIGGER_PK 1 /* is a trigger on the PK relation */
161-
#define RI_TRIGGER_FK 2 /* is a trigger on the FK relation */
162-
#define RI_TRIGGER_NONE 0 /* is not an RI trigger function */
163-
164-
165159
static List *MergeAttributes(List *schema, List *supers, bool istemp,
166160
List **supOids, List **supconstr, int *supOidCount);
167161
static bool change_varattnos_of_a_node(Node *node, const AttrNumber *newattno);
@@ -246,7 +240,6 @@ static void ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel,
246240
char *tablespacename);
247241
static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace);
248242
static void copy_relation_data(Relation rel, SMgrRelation dst);
249-
static int ri_trigger_type(Oid tgfoid);
250243
static void update_ri_trigger_args(Oid relid,
251244
const char *oldname,
252245
const char *newname,
@@ -1571,39 +1564,6 @@ renamerel(Oid myrelid, const char *newrelname)
15711564
relation_close(targetrelation, NoLock);
15721565
}
15731566

1574-
1575-
/*
1576-
* Given a trigger function OID, determine whether it is an RI trigger,
1577-
* and if so whether it is attached to PK or FK relation.
1578-
*
1579-
* XXX this probably doesn't belong here; should be exported by
1580-
* ri_triggers.c
1581-
*/
1582-
static int
1583-
ri_trigger_type(Oid tgfoid)
1584-
{
1585-
switch (tgfoid)
1586-
{
1587-
case F_RI_FKEY_CASCADE_DEL:
1588-
case F_RI_FKEY_CASCADE_UPD:
1589-
case F_RI_FKEY_RESTRICT_DEL:
1590-
case F_RI_FKEY_RESTRICT_UPD:
1591-
case F_RI_FKEY_SETNULL_DEL:
1592-
case F_RI_FKEY_SETNULL_UPD:
1593-
case F_RI_FKEY_SETDEFAULT_DEL:
1594-
case F_RI_FKEY_SETDEFAULT_UPD:
1595-
case F_RI_FKEY_NOACTION_DEL:
1596-
case F_RI_FKEY_NOACTION_UPD:
1597-
return RI_TRIGGER_PK;
1598-
1599-
case F_RI_FKEY_CHECK_INS:
1600-
case F_RI_FKEY_CHECK_UPD:
1601-
return RI_TRIGGER_FK;
1602-
}
1603-
1604-
return RI_TRIGGER_NONE;
1605-
}
1606-
16071567
/*
16081568
* Scan pg_trigger for RI triggers that are on the specified relation
16091569
* (if fk_scan is false) or have it as the tgconstrrel (if fk_scan
@@ -1663,7 +1623,7 @@ update_ri_trigger_args(Oid relid,
16631623
const char *arga[RI_MAX_ARGUMENTS];
16641624
const char *argp;
16651625

1666-
tg_type = ri_trigger_type(pg_trigger->tgfoid);
1626+
tg_type = RI_FKey_trigger_type(pg_trigger->tgfoid);
16671627
if (tg_type == RI_TRIGGER_NONE)
16681628
{
16691629
/* Not an RI trigger, forget it */

‎src/backend/commands/trigger.c

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.188 2005/05/06 17:24:53 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.189 2005/05/30 07:20:58 neilc Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -2994,54 +2994,47 @@ AfterTriggerSaveEvent(ResultRelInfo *relinfo, int event, bool row_trigger,
29942994
continue;
29952995

29962996
/*
2997-
* If it is an RI UPDATE trigger, and the referenced keys have
2998-
* not changed, short-circuit queuing of the event; there's no
2999-
* need to fire the trigger.
2997+
* If this is an UPDATE of a PK table or FK table that does
2998+
* not change the PK or FK respectively, we can skip queuing
2999+
* the event: there is no need to fire the trigger.
30003000
*/
30013001
if ((event & TRIGGER_EVENT_OPMASK) == TRIGGER_EVENT_UPDATE)
30023002
{
3003-
bool is_ri_trigger;
3004-
3005-
switch (trigger->tgfoid)
3003+
switch (RI_FKey_trigger_type(trigger->tgfoid))
30063004
{
3007-
case F_RI_FKEY_NOACTION_UPD:
3008-
case F_RI_FKEY_CASCADE_UPD:
3009-
case F_RI_FKEY_RESTRICT_UPD:
3010-
case F_RI_FKEY_SETNULL_UPD:
3011-
case F_RI_FKEY_SETDEFAULT_UPD:
3012-
is_ri_trigger = true;
3005+
case RI_TRIGGER_PK:
3006+
/* Update on PK table */
3007+
if (RI_FKey_keyequal_upd_pk(trigger, rel, oldtup, newtup))
3008+
{
3009+
/* key unchanged, so skip queuing this event */
3010+
continue;
3011+
}
30133012
break;
30143013

3015-
default:
3016-
is_ri_trigger = false;
3014+
case RI_TRIGGER_FK:
3015+
/*
3016+
* Update on FK table
3017+
*
3018+
* There is one exception when updating FK tables:
3019+
* if the updated row was inserted by our own
3020+
* transaction and the FK is deferred, we still
3021+
* need to fire the trigger. This is because our
3022+
* UPDATE will invalidate the INSERT so the
3023+
* end-of-transaction INSERT RI trigger will not
3024+
* do anything, so we have to do the check for the
3025+
* UPDATE anyway.
3026+
*/
3027+
if (HeapTupleHeaderGetXmin(oldtup->t_data) !=
3028+
GetCurrentTransactionId() &&
3029+
RI_FKey_keyequal_upd_fk(trigger, rel, oldtup, newtup))
3030+
{
3031+
continue;
3032+
}
30173033
break;
3018-
}
30193034

3020-
if (is_ri_trigger)
3021-
{
3022-
TriggerData LocTriggerData;
3023-
3024-
LocTriggerData.type = T_TriggerData;
3025-
LocTriggerData.tg_event =
3026-
TRIGGER_EVENT_UPDATE | TRIGGER_EVENT_ROW;
3027-
LocTriggerData.tg_relation = rel;
3028-
LocTriggerData.tg_trigtuple = oldtup;
3029-
LocTriggerData.tg_newtuple = newtup;
3030-
LocTriggerData.tg_trigger = trigger;
3031-
/*
3032-
* We do not currently know which buffers the passed tuples
3033-
* are in, but it does not matter because RI_FKey_keyequal_upd
3034-
* does not care. We could expand the API of this function
3035-
* if it becomes necessary to set these fields accurately.
3036-
*/
3037-
LocTriggerData.tg_trigtuplebuf = InvalidBuffer;
3038-
LocTriggerData.tg_newtuplebuf = InvalidBuffer;
3039-
3040-
if (RI_FKey_keyequal_upd(&LocTriggerData))
3041-
{
3042-
/* key unchanged, so skip queuing this event */
3043-
continue;
3044-
}
3035+
case RI_TRIGGER_NONE:
3036+
/* Not an FK trigger */
3037+
break;
30453038
}
30463039
}
30473040

0 commit comments

Comments
 (0)