8000 Change FK trigger creation order to better support self-referential FKs. · jaylevitt/postgres@8a1689d · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 8a1689d

Browse files
tglsfdctglsfdc
committed
Change FK trigger creation order to better support self-referential FKs.
When a foreign-key constraint references another column of the same table, row updates will queue both the PK's ON UPDATE action and the FK's CHECK action in the same event. The ON UPDATE action must execute first, else the CHECK will check a non-final state of the row and possibly throw an inappropriate error, as seen in bug #6268 from Roman Lytovchenko. Now, the firing order of multiple triggers for the same event is determined by the sort order of their pg_trigger.tgnames, and the auto-generated names we use for FK triggers are "RI_ConstraintTrigger_NNNN" where NNNN is the trigger OID. So most of the time the firing order is the same as creation order, and so rearranging the creation order fixes it. This patch will fail to fix the problem if the OID counter wraps around or adds a decimal digit (eg, from 99999 to 100000) while we are creating the triggers for an FK constraint. Given the small odds of that, and the low usage of self-referential FKs, we'll live with that solution in the back branches. A better fix is to change the auto-generated names for FK triggers, but it seems unwise to do that in stable branches because there may be client code that depends on the naming convention. We'll fix it that way in HEAD in a separate patch. Back-patch to all supported branches, since this bug has existed for a long time.
1 parent 4911a27 commit 8a1689d

File tree

3 files changed

+75
-7
lines changed

3 files changed

+75
-7
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6301,13 +6301,6 @@ createForeignKeyTriggers(Relation rel, Constraint *fkconstraint,
63016301
/* Make changes-so-far visible */
63026302
CommandCounterIncrement();
63036303

6304-
/*
6305-
* Build and execute a CREATE CONSTRAINT TRIGGER statement for the CHECK
6306-
* action for both INSERTs and UPDATEs on the referencing table.
6307-
*/
6308-
CreateFKCheckTrigger(myRel, fkconstraint, constraintOid, indexOid, true);
6309-
CreateFKCheckTrigger(myRel, fkconstraint, constraintOid, indexOid, false);
6310-
63116304
/*
63126305
* Build and execute a CREATE CONSTRAINT TRIGGER statement for the ON
63136306
* DELETE action on the referenced table.
@@ -6410,6 +6403,27 @@ createForeignKeyTriggers(Relation rel, Constraint *fkconstraint,
64106403
fk_trigger->args = NIL;
64116404

64126405
(void) CreateTrigger(fk_trigger, NULL, constraintOid, indexOid, true);
6406+
6407+
/* Make changes-so-far visible */
6408+
CommandCounterIncrement();
6409+
6410+
/*
6411+
* Build and execute CREATE CONSTRAINT TRIGGER statements for the CHECK
6412+
* action for both INSERTs and UPDATEs on the referencing table.
6413+
*
6414+
* Note: for a self-referential FK (referencing and referenced tables are
6415+
* the same), it is important that the ON UPDATE action fires before the
6416+
* CHECK action, since both triggers will fire on the same row during an
6417+
* UPDATE event; otherwise the CHECK trigger will be checking a non-final
6418+
* state of the row. Because triggers fire in name order, we are
6419+
* effectively relying on the OIDs of the triggers to sort correctly as
6420+
* text. This will work except when the OID counter wraps around or adds
6421+
* a digit, eg "99999" sorts after "100000". That is infrequent enough,
6422+
* and the use of self-referential FKs is rare enough, that we live with
6423+
* it for now. There will be a real fix in PG 9.2.
6424+
*/
6425+
CreateFKCheckTrigger(myRel, fkconstraint, constraintOid, indexOid, true);
6426+
CreateFKCheckTrigger(myRel, fkconstraint, constraintOid, indexOid, false);
64136427
}
64146428

64156429
/*

‎src/test/regress/expected/foreign_key.out

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,3 +1287,35 @@ SELECT * FROM tasks;
12871287
(3 rows)
12881288

12891289
COMMIT;
1290+
--
1291+
-- Test self-referential FK with CASCADE (bug #6268)
1292+
--
1293+
create temp table selfref (
1294+
a int primary key,
1295+
b int,
1296+
foreign key (b) references selfref (a)
1297+
on update cascade on delete cascade
1298+
);
1299+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "selfref_pkey" for table "selfref"
1300+
insert into selfref (a, b)
1301+
values
1302+
(0, 0),
1303+
(1, 1);
1304+
begin;
1305+
update selfref set a = 123 where a = 0;
1306+
select a, b from selfref;
1307+
a | b
1308+
-----+-----
1309+
1 | 1
1310+
123 | 123
1311+
(2 rows)
1312+
1313+
update selfref set a = 456 where a = 123;
1314+
select a, b from selfref;
1315+
a | b
1316+
-----+-----
1317+
1 | 1
1318+
456 | 456
1319+
(2 rows)
1320+
1321+
commit;

‎src/test/regress/sql/foreign_key.sql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,3 +921,25 @@ SELECT * FROM tasks;
921921
DELETE FROM users WHERE id = 2;
922922
SELECT * FROM tasks;
923923
COMMIT;
924+
925+
--
926+
-- Test self-referential FK with CASCADE (bug #6268)
927+
--
928+
create temp table selfref (
929+
a int primary key,
930+
b int,
931+
foreign key (b) references selfref (a)
932+
on update cascade on delete cascade
933+
);
934+
935+
insert into selfref (a, b)
936+
values
937+
(0, 0),
938+
(1, 1);
939+
940+
begin;
941+
update selfref set a = 123 where a = 0;
942+
select a, b from selfref;
943+
update selfref set a = 456 where a = 123;
944+
select a, b from selfref;
945+
commit;

0 commit comments

Comments
 (0)
0