projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 9a743ff)
Change FK trigger creation order to better support self-referential FKs.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 26 Oct 2011 17:02:58 +0000 (13:02 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 26 Oct 2011 17:02:58 +0000 (13:02 -0400)
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.

src/backend/commands/tablecmds.c patch | blob | blame | history
src/test/regress/expected/foreign_key.out patch | blob | blame | history
src/test/regress/sql/foreign_key.sql patch | blob | blame | history

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 48bd6fa4419b0924e8258e16c42bb4f8c8dd9814..e523827a0ab7a64a0503eb91384246c100a5ba41 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -4735,13 +4735,6 @@ createForeignKeyTriggers(Relation rel, FkConstraint *fkconstraint,
    /* Make changes-so-far visible */
    CommandCounterIncrement();
 
-   /*
-    * Build and execute a CREATE CONSTRAINT TRIGGER statement for the CHECK
-    * action for both INSERTs and UPDATEs on the referencing table.
-    */
-   CreateFKCheckTrigger(myRel, fkconstraint, &constrobj, &trigobj, true);
-   CreateFKCheckTrigger(myRel, fkconstraint, &constrobj, &trigobj, false);
-
    /*
     * Build and execute a CREATE CONSTRAINT TRIGGER statement for the ON
     * DELETE action on the referenced table.
@@ -4879,6 +4872,27 @@ createForeignKeyTriggers(Relation rel, FkConstraint *fkconstraint,
 
    /* Register dependency from trigger to constraint */
    recordDependencyOn(&trigobj, &constrobj, DEPENDENCY_INTERNAL);
+
+   /* Make changes-so-far visible */
+   CommandCounterIncrement();
+
+   /*
+    * Build and execute CREATE CONSTRAINT TRIGGER statements for the CHECK
+    * action for both INSERTs and UPDATEs on the referencing table.
+    *
+    * Note: for a self-referential FK (referencing and referenced tables are
+    * the same), it is important that the ON UPDATE action fires before the
+    * CHECK action, since both triggers will fire on the same row during an
+    * UPDATE event; otherwise the CHECK trigger will be checking a non-final
+    * state of the row.  Because triggers fire in name order, we are
+    * effectively relying on the OIDs of the triggers to sort correctly as
+    * text.  This will work except when the OID counter wraps around or adds
+    * a digit, eg "99999" sorts after "100000".  That is infrequent enough,
+    * and the use of self-referential FKs is rare enough, that we live with
+    * it for now.  There will be a real fix in PG 9.2.
+    */
+   CreateFKCheckTrigger(myRel, fkconstraint, &constrobj, &trigobj, true);
+   CreateFKCheckTrigger(myRel, fkconstraint, &constrobj, &trigobj, false);
 }
 
 /*
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 0807423597c73750b25f82f8803b62147fbf2610..589ee337b15f25311b3c9d75b24ce7dc56a35da6 100644 (file)
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1276,3 +1276,35 @@ SELECT * FROM tasks;
 (3 rows)
 
 COMMIT;
+--
+-- Test self-referential FK with CASCADE (bug #6268)
+--
+create temp table selfref (
+    a int primary key,
+    b int,
+    foreign key (b) references selfref (a)
+        on update cascade on delete cascade
+);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "selfref_pkey" for table "selfref"
+insert into selfref (a, b)
+values
+    (0, 0),
+    (1, 1);
+begin;
+    update selfref set a = 123 where a = 0;
+    select a, b from selfref;
+  a  |  b  
+-----+-----
+   1 |   1
+ 123 | 123
+(2 rows)
+
+    update selfref set a = 456 where a = 123;
+    select a, b from selfref;
+  a  |  b  
+-----+-----
+   1 |   1
+ 456 | 456
+(2 rows)
+
+commit;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 0135368db8b74a1a6e0c0ae783054041874d0f55..1d92c91e7450af6f2b886cc8ee0e8fdda3b7834a 100644 (file)
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -899,3 +899,25 @@ SELECT * FROM tasks;
 DELETE FROM users WHERE id = 2;
 SELECT * FROM tasks;
 COMMIT;
+
+--
+-- Test self-referential FK with CASCADE (bug #6268)
+--
+create temp table selfref (
+    a int primary key,
+    b int,
+    foreign key (b) references selfref (a)
+        on update cascade on delete cascade
+);
+
+insert into selfref (a, b)
+values
+    (0, 0),
+    (1, 1);
+
+begin;
+    update selfref set a = 123 where a = 0;
+    select a, b from selfref;
+    update selfref set a = 456 where a = 123;
+    select a, b from selfref;
+commit;
This is the main PostgreSQL git repository.