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/commands/trigger.c b/src/backend/commands/trigger.c
index 2ccc75f8ff299a7909e8b8945738d63072cc4870..6a6381b485b1c1fb08c1505dce621a697ebe6bd0 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3338,8 +3338,7 @@ AfterTriggerSaveEvent(ResultRelInfo *relinfo, int event, bool row_trigger,
                                         * anything, so we have to do the check for the UPDATE
                                         * anyway.
                                         */
-                                       if (HeapTupleHeaderGetXmin(oldtup->t_data) !=
-                                               GetCurrentTransactionId() &&
+                                       if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(oldtup->t_data)) &&
                                                RI_FKey_keyequal_upd_fk(trigger, rel, oldtup, newtup))
                                        {
                                                continue;

diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 5424731d4d0b0c96dbd31499d9b771a150c761e7..6bd4c42577aac7197f97adf8a41763c7f7a996e8 100644 (file)
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1172,3 +1172,40 @@ UPDATE fktable SET id = id + 1;
 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 2b22d0cecd1d8c714adcb932a11a3f26cc7b97ca..78a26ed025b3a4a51f8577c49958980be66af64e 100644 (file)
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -808,3 +808,52 @@ 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;