From b1def736b00595c8983cee5d513762f084ce444b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 17 Jul 2007 17:45:48 +0000 Subject: [PATCH] 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. --- src/backend/commands/trigger.c | 5 +-- src/test/regress/expected/foreign_key.out | 37 +++++++++++++++++ src/test/regress/sql/foreign_key.sql | 49 +++++++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 86af435116b..739777100c2 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.195.2.2 2006/01/12 21:49:06 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.195.2.3 2007/07/17 17:45:48 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3142,8 +3142,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 e3ea729e6fa..5ad23acaf93 100644 --- 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 5ab15c1b39b..cc3f9abfaaf 100644 --- 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;