Fix incorrect optimization of foreign-key checks. When an UPDATE on the

referencing table does not change the tuple's FK column(s), we don't bother
to check the PK table since the constraint was presumably already valid.
However, the check is still necessary if the tuple was inserted by our own
transaction, since in that case the INSERT trigger will conclude it need not
make the check (since its version of the tuple has been deleted).  We got this
right for simple cases, but not when the insert and update are in different
subtransactions of the current top-level transaction; in such cases the FK
check would never be made at all.  (Hence, problem dates back to 8.0 when
subtransactions were added --- it's actually the subtransaction version of a
bug fixed in 7.3.5.)  Fix, and add regression test cases.  Report and fix by
Affan Salman.

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 56aae272410741d9215037c1780e936db7acfcd6..5a7b46bff9613ae5fb9a6f0b877f6883abcf5b70 100644 (file)
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -377,12 +377,13 @@ RI_FKey_check(PG_FUNCTION_ARGS)
 
        /*
         * No need to check anything if old and new references are the same on
-        * UPDATE.
+        * UPDATE; unless the updated row was inserted by our own transaction.
+        * This is because the UPDATE invalidated the INSERT, so the INSERT
+        * trigger won't have checked.
         */
        if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
        {
-               if (HeapTupleHeaderGetXmin(old_row->t_data) !=
-                       GetCurrentTransactionId() &&
+               if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)) &&
                        ri_KeysEqual(fk_rel, old_row, new_row, &qkey,
                                                 RI_KEYPAIR_FK_IDX))
                {

diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index ae80b33c6f74b0f009b34a1b814f9f279b93d638..a479f4932a82ab587c160c94d3afe46189236c48 100644 (file)
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1128,3 +1128,82 @@ WARNING:  foreign key constraint "fk_241_132" will require costly sequential sca
 DETAIL:  Key columns "x2" and "id1" are of different types: character varying and integer.
 WARNING:  foreign key constraint "fk_241_132" will require costly sequential scans
 DETAIL:  Key columns "x4" and "id3" are of different types: text and real.
+DROP TABLE pktable, fktable CASCADE;
+NOTICE:  drop cascades to constraint fk_241_132 on table fktable
+NOTICE:  drop cascades to constraint fk_123_231 on table fktable
+NOTICE:  drop cascades to constraint fk_253_213 on table fktable
+NOTICE:  drop cascades to constraint fk_213_213 on table fktable
+NOTICE:  drop cascades to constraint fk_123_123 on table fktable
+NOTICE:  drop cascades to constraint fk_5_1 on table fktable
+NOTICE:  drop cascades to constraint fk_3_1 on table fktable
+NOTICE:  drop cascades to constraint fk_2_1 on table fktable
+NOTICE:  drop cascades to constraint fktable_x1_fkey on table fktable
+NOTICE:  drop cascades to constraint fk_4_2 on table fktable
+NOTICE:  drop cascades to constraint fk_1_2 on table fktable
+NOTICE:  drop cascades to constraint fktable_x2_fkey on table fktable
+NOTICE:  drop cascades to constraint fk_1_3 on table fktable
+NOTICE:  drop cascades to constraint fk_2_3 on table fktable
+NOTICE:  drop cascades to constraint fktable_x3_fkey on table fktable
+-- test a tricky case: we can elide firing the FK check trigger during
+-- an UPDATE if the UPDATE did not change the foreign key
+-- field. However, we can't do this if our transaction was the one that
+-- created the updated row and the trigger is deferred, since our UPDATE
+-- will have invalidated the original newly-inserted tuple, and therefore
+-- cause the on-INSERT RI trigger not to be fired.
+CREATE TEMP TABLE pktable (
+    id int primary key,
+    other int
+);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "pktable_pkey" for table "pktable"
+CREATE TEMP TABLE fktable (
+    id int primary key,
+    fk int references pktable deferrable initially deferred
+);
+NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "fktable_pkey" for table "fktable"
+INSERT INTO pktable VALUES (5, 10);
+BEGIN;
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+-- should catch error from initial INSERT
+COMMIT;
+ERROR:  insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey"
+DETAIL:  Key (fk)=(20) is not present in table "pktable".
+-- check same case when insert is in a different subtransaction than update
+BEGIN;
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+-- UPDATE will be in a subxact
+SAVEPOINT savept1;
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+-- should catch error from initial INSERT
+COMMIT;
+ERROR:  insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey"
+DETAIL:  Key (fk)=(20) is not present in table "pktable".
+BEGIN;
+-- INSERT will be in a subxact
+SAVEPOINT savept1;
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+RELEASE SAVEPOINT savept1;
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+-- should catch error from initial INSERT
+COMMIT;
+ERROR:  insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey"
+DETAIL:  Key (fk)=(20) is not present in table "pktable".
+BEGIN;
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+-- UPDATE will be in a subxact
+SAVEPOINT savept1;
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+-- Roll back the UPDATE
+ROLLBACK TO savept1;
+-- should catch error from initial INSERT
+COMMIT;
+ERROR:  insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey"
+DETAIL:  Key (fk)=(20) is not present in table "pktable".

diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index ad5865683fb252333735b8b1a1ed2430d5e161e1..dee046a1952a845c4e4e4e99689040d8a8cf6431 100644 (file)
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -774,3 +774,84 @@ FOREIGN KEY (x1,x2,x3) REFERENCES pktable(id2,id3,id1);
 
 ALTER TABLE fktable ADD CONSTRAINT fk_241_132
 FOREIGN KEY (x2,x4,x1) REFERENCES pktable(id1,id3,id2);
+
+DROP TABLE pktable, fktable CASCADE;
+
+-- test a tricky case: we can elide firing the FK check trigger during
+-- an UPDATE if the UPDATE did not change the foreign key
+-- field. However, we can't do this if our transaction was the one that
+-- created the updated row and the trigger is deferred, since our UPDATE
+-- will have invalidated the original newly-inserted tuple, and therefore
+-- cause the on-INSERT RI trigger not to be fired.
+
+CREATE TEMP TABLE pktable (
+    id int primary key,
+    other int
+);
+
+CREATE TEMP TABLE fktable (
+    id int primary key,
+    fk int references pktable deferrable initially deferred
+);
+
+INSERT INTO pktable VALUES (5, 10);
+
+BEGIN;
+
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+
+-- should catch error from initial INSERT
+COMMIT;
+
+-- check same case when insert is in a different subtransaction than update
+
+BEGIN;
+
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+
+-- UPDATE will be in a subxact
+SAVEPOINT savept1;
+
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+
+-- should catch error from initial INSERT
+COMMIT;
+
+BEGIN;
+
+-- INSERT will be in a subxact
+SAVEPOINT savept1;
+
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+
+RELEASE SAVEPOINT savept1;
+
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+
+-- should catch error from initial INSERT
+COMMIT;
+
+BEGIN;
+
+-- doesn't match PK, but no error yet
+INSERT INTO fktable VALUES (0, 20);
+
+-- UPDATE will be in a subxact
+SAVEPOINT savept1;
+
+-- don't change FK
+UPDATE fktable SET id = id + 1;
+
+-- Roll back the UPDATE
+ROLLBACK TO savept1;
+
+-- should catch error from initial INSERT
+COMMIT;