From a4820434c1a62e0c5f4051a31ad8b4a11f0a6ad7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 3 May 2015 11:30:24 -0400 Subject: [PATCH] Fix overlooked relcache invalidation in ALTER TABLE ... ALTER CONSTRAINT. When altering the deferredness state of a foreign key constraint, we correctly updated the catalogs and then invalidated the relcache state for the target relation ... but that's not the only relation with relevant triggers. Must invalidate the other table as well, or the state change fails to take effect promptly for operations triggered on the other table. Per bug #13224 from Christian Ullrich. In passing, reorganize regression test case for this feature so that it isn't randomly injected into the middle of an unrelated test sequence. Oversight in commit f177cbfe676dc2c7ca2b206c54d6bf819feeea8b. Back-patch to 9.4 where the faulty code was added. --- src/backend/commands/tablecmds.c | 22 +++++++++-- src/test/regress/expected/foreign_key.out | 43 ++++++++++++--------- src/test/regress/sql/foreign_key.sql | 47 +++++++++++++---------- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 5d84285752..299d8ccd81 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -6603,12 +6603,12 @@ static ObjectAddress ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode) { + Constraint *cmdcon; Relation conrel; SysScanDesc scan; ScanKeyData key; HeapTuple contuple; Form_pg_constraint currcon = NULL; - Constraint *cmdcon = NULL; bool found = false; ObjectAddress address; @@ -6655,10 +6655,11 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, HeapTuple copyTuple; HeapTuple tgtuple; Form_pg_constraint copy_con; - Form_pg_trigger copy_tg; + List *otherrelids = NIL; ScanKeyData tgkey; SysScanDesc tgscan; Relation tgrel; + ListCell *lc; /* * Now update the catalog, while we have the door open. @@ -6691,8 +6692,16 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, while (HeapTupleIsValid(tgtuple = systable_getnext(tgscan))) { + Form_pg_trigger copy_tg; + copyTuple = heap_copytuple(tgtuple); copy_tg = (Form_pg_trigger) GETSTRUCT(copyTuple); + + /* Remember OIDs of other relation(s) involved in FK constraint */ + if (copy_tg->tgrelid != RelationGetRelid(rel)) + otherrelids = list_append_unique_oid(otherrelids, + copy_tg->tgrelid); + copy_tg->tgdeferrable = cmdcon->deferrable; copy_tg->tginitdeferred = cmdcon->initdeferred; simple_heap_update(tgrel, ©Tuple->t_self, copyTuple); @@ -6709,9 +6718,16 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, heap_close(tgrel, RowExclusiveLock); /* - * Invalidate relcache so that others see the new attributes. + * Invalidate relcache so that others see the new attributes. We must + * inval both the named rel and any others having relevant triggers. + * (At present there should always be exactly one other rel, but + * there's no need to hard-wire such an assumption here.) */ CacheInvalidateRelcache(rel); + foreach(lc, otherrelids) + { + CacheInvalidateRelcacheByRelid(lfirst_oid(lc)); + } ObjectAddressSet(address, ConstraintRelationId, HeapTupleGetOid(contuple)); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 0299bfe873..8c47babb6d 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1132,15 +1132,6 @@ CREATE TEMP TABLE fktable ( id int primary key, fk int references pktable deferrable initially deferred ); --- check ALTER CONSTRAINT -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE; --- illegal option -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY DEFERRED; -ERROR: constraint declared INITIALLY DEFERRED must be DEFERRABLE -LINE 1: ...e ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY ... - ^ --- reset -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY DEFERRED; INSERT INTO pktable VALUES (5, 10); BEGIN; -- doesn't match PK, but no error yet @@ -1151,16 +1142,6 @@ 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". --- change the constraint definition and retest -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY IMMEDIATE; -BEGIN; --- doesn't match PK, should throw error now -INSERT INTO fktable VALUES (0, 20); -ERROR: insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey" -DETAIL: Key (fk)=(20) is not present in table "pktable". -COMMIT; --- reset -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY DEFERRED; -- check same case when insert is in a different subtransaction than update BEGIN; -- doesn't match PK, but no error yet @@ -1198,6 +1179,30 @@ ROLLBACK TO savept1; 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 ALTER CONSTRAINT +-- +INSERT INTO fktable VALUES (1, 5); +ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY IMMEDIATE; +BEGIN; +-- doesn't match FK, should throw error now +UPDATE pktable SET id = 10 WHERE id = 5; +ERROR: update or delete on table "pktable" violates foreign key constraint "fktable_fk_fkey" on table "fktable" +DETAIL: Key (id)=(5) is still referenced from table "fktable". +COMMIT; +BEGIN; +-- doesn't match PK, should throw error now +INSERT INTO fktable VALUES (0, 20); +ERROR: insert or update on table "fktable" violates foreign key constraint "fktable_fk_fkey" +DETAIL: Key (fk)=(20) is not present in table "pktable". +COMMIT; +-- try additional syntax +ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE; +-- illegal option +ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY DEFERRED; +ERROR: constraint declared INITIALLY DEFERRED must be DEFERRABLE +LINE 1: ...e ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY ... + ^ -- test order of firing of FK triggers when several RI-induced changes need to -- be made to the same row. This was broken by subtransaction-related -- changes in 8.0. diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 531c881f63..53276e4d67 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -818,13 +818,6 @@ CREATE TEMP TABLE fktable ( fk int references pktable deferrable initially deferred ); --- check ALTER CONSTRAINT -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE; --- illegal option -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY DEFERRED; --- reset -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY DEFERRED; - INSERT INTO pktable VALUES (5, 10); BEGIN; @@ -838,19 +831,6 @@ UPDATE fktable SET id = id + 1; -- should catch error from initial INSERT COMMIT; --- change the constraint definition and retest -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY IMMEDIATE; - -BEGIN; - --- doesn't match PK, should throw error now -INSERT INTO fktable VALUES (0, 20); - -COMMIT; - --- reset -ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY DEFERRED; - -- check same case when insert is in a different subtransaction than update BEGIN; @@ -900,6 +880,33 @@ ROLLBACK TO savept1; -- should catch error from initial INSERT COMMIT; +-- +-- check ALTER CONSTRAINT +-- + +INSERT INTO fktable VALUES (1, 5); + +ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey DEFERRABLE INITIALLY IMMEDIATE; + +BEGIN; + +-- doesn't match FK, should throw error now +UPDATE pktable SET id = 10 WHERE id = 5; + +COMMIT; + +BEGIN; + +-- doesn't match PK, should throw error now +INSERT INTO fktable VALUES (0, 20); + +COMMIT; + +-- try additional syntax +ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE; +-- illegal option +ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY DEFERRED; + -- test order of firing of FK triggers when several RI-induced changes need to -- be made to the same row. This was broken by subtransaction-related -- changes in 8.0.