Skip to content

Commit b53c841

Browse files
tglsfdctglsfdc
committed
Fix incorrect trigger-property updating in ALTER CONSTRAINT.
The code to change the deferrability properties of a foreign-key constraint updated all the associated triggers to match; but a moment's examination of the code that creates those triggers in the first place shows that only some of them should track the constraint's deferrability properties. This leads to odd failures in subsequent exercise of the foreign key, as the triggers are fired at the wrong times. Fix that, and add a regression test comparing the trigger properties produced by ALTER CONSTRAINT with those you get by creating the constraint as-intended to begin with. Per report from James Parks. Back-patch to 9.4 where this ALTER functionality was introduced. Report: <CAJ3Xv+jzJ8iNNUcp4RKW8b6Qp1xVAxHwSXVpjBNygjKxcVuE9w@mail.gmail.com>
1 parent 59f5b61 commit b53c841

File tree

3 files changed

+119
-5
lines changed

3 files changed

+119
-5
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6710,16 +6710,34 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
67106710

67116711
while (HeapTupleIsValid(tgtuple = systable_getnext(tgscan)))
67126712
{
6713+
Form_pg_trigger tgform = (Form_pg_trigger) GETSTRUCT(tgtuple);
67136714
Form_pg_trigger copy_tg;
67146715

6716+
/*
6717+
* Remember OIDs of other relation(s) involved in FK constraint.
6718+
* (Note: it's likely that we could skip forcing a relcache inval
6719+
* for other rels that don't have a trigger whose properties
6720+
* change, but let's be conservative.)
6721+
*/
6722+
if (tgform->tgrelid != RelationGetRelid(rel))
6723+
otherrelids = list_append_unique_oid(otherrelids,
6724+
tgform->tgrelid);
6725+
6726+
/*
6727+
* Update deferrability of RI_FKey_noaction_del,
6728+
* RI_FKey_noaction_upd, RI_FKey_check_ins and RI_FKey_check_upd
6729+
* triggers, but not others; see createForeignKeyTriggers and
6730+
* CreateFKCheckTrigger.
6731+
*/
6732+
if (tgform->tgfoid != F_RI_FKEY_NOACTION_DEL &&
6733+
tgform->tgfoid != F_RI_FKEY_NOACTION_UPD &&
6734+
tgform->tgfoid != F_RI_FKEY_CHECK_INS &&
6735+
tgform->tgfoid != F_RI_FKEY_CHECK_UPD)
6736+
continue;
6737+
67156738
copyTuple = heap_copytuple(tgtuple);
67166739
copy_tg = (Form_pg_trigger) GETSTRUCT(copyTuple);
67176740

6718-
/* Remember OIDs of other relation(s) involved in FK constraint */
6719-
if (copy_tg->tgrelid != RelationGetRelid(rel))
6720-
otherrelids = list_append_unique_oid(otherrelids,
6721-
copy_tg->tgrelid);
6722-
67236741
copy_tg->tgdeferrable = cmdcon->deferrable;
67246742
copy_tg->tginitdeferred = cmdcon->initdeferred;
67256743
simple_heap_update(tgrel, &copyTuple->t_self, copyTuple);
@@ -7481,6 +7499,9 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint,
74817499

74827500
/*
74837501
* Create the triggers that implement an FK constraint.
7502+
*
7503+
* NB: if you change any trigger properties here, see also
7504+
* ATExecAlterConstraint.
74847505
*/
74857506
static void
74867507
createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,

‎src/test/regress/expected/alter_table.out

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,66 @@ ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest2, ftest1)
525525
references pktable(ptest1, ptest2);
526526
ERROR: foreign key constraint "fktable_ftest2_fkey" cannot be implemented
527527
DETAIL: Key columns "ftest2" and "ptest1" are of incompatible types: inet and integer.
528+
DROP TABLE FKTABLE;
529+
DROP TABLE PKTABLE;
530+
-- Test that ALTER CONSTRAINT updates trigger deferrability properly
531+
CREATE TEMP TABLE PKTABLE (ptest1 int primary key);
532+
CREATE TEMP TABLE FKTABLE (ftest1 int);
533+
ALTER TABLE FKTABLE ADD CONSTRAINT fknd FOREIGN KEY(ftest1) REFERENCES pktable
534+
ON DELETE CASCADE ON UPDATE NO ACTION NOT DEFERRABLE;
535+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdd FOREIGN KEY(ftest1) REFERENCES pktable
536+
ON DELETE CASCADE ON UPDATE NO ACTION DEFERRABLE INITIALLY DEFERRED;
537+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdi FOREIGN KEY(ftest1) REFERENCES pktable
538+
ON DELETE CASCADE ON UPDATE NO ACTION DEFERRABLE INITIALLY IMMEDIATE;
539+
ALTER TABLE FKTABLE ADD CONSTRAINT fknd2 FOREIGN KEY(ftest1) REFERENCES pktable
540+
ON DELETE CASCADE ON UPDATE NO ACTION DEFERRABLE INITIALLY DEFERRED;
541+
ALTER TABLE FKTABLE ALTER CONSTRAINT fknd2 NOT DEFERRABLE;
542+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdd2 FOREIGN KEY(ftest1) REFERENCES pktable
543+
ON DELETE CASCADE ON UPDATE NO ACTION NOT DEFERRABLE;
544+
ALTER TABLE FKTABLE ALTER CONSTRAINT fkdd2 DEFERRABLE INITIALLY DEFERRED;
545+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdi2 FOREIGN KEY(ftest1) REFERENCES pktable
546+
ON DELETE CASCADE ON UPDATE NO ACTION NOT DEFERRABLE;
547+
ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 DEFERRABLE INITIALLY IMMEDIATE;
548+
SELECT conname, tgfoid::regproc, tgtype, tgdeferrable, tginitdeferred
549+
FROM pg_trigger JOIN pg_constraint con ON con.oid = tgconstraint
550+
WHERE tgrelid = 'pktable'::regclass
551+
ORDER BY 1,2,3;
552+
conname | tgfoid | tgtype | tgdeferrable | tginitdeferred
553+
---------+------------------------+--------+--------------+----------------
554+
fkdd | "RI_FKey_cascade_del" | 9 | f | f
555+
fkdd | "RI_FKey_noaction_upd" | 17 | t | t
556+
fkdd2 | "RI_FKey_cascade_del" | 9 | f | f
557+
fkdd2 | "RI_FKey_noaction_upd" | 17 | t | t
558+
fkdi | "RI_FKey_cascade_del" | 9 | f | f
559+
fkdi | "RI_FKey_noaction_upd" | 17 | t | f
560+
fkdi2 | "RI_FKey_cascade_del" | 9 | f | f
561+
fkdi2 | "RI_FKey_noaction_upd" | 17 | t | f
562+
fknd | "RI_FKey_cascade_del" | 9 | f | f
563+
fknd | "RI_FKey_noaction_upd" | 17 | f | f
564+
fknd2 | "RI_FKey_cascade_del" | 9 | f | f
565+
fknd2 | "RI_FKey_noaction_upd" | 17 | f | f
566+
(12 rows)
567+
568+
SELECT conname, tgfoid::regproc, tgtype, tgdeferrable, tginitdeferred
569+
FROM pg_trigger JOIN pg_constraint con ON con.oid = tgconstraint
570+
WHERE tgrelid = 'fktable'::regclass
571+
ORDER BY 1,2,3;
572+
conname | tgfoid | tgtype | tgdeferrable | tginitdeferred
573+
---------+---------------------+--------+--------------+----------------
574+
fkdd | "RI_FKey_check_ins" | 5 | t | t
575+
fkdd | "RI_FKey_check_upd" | 17 | t | t
576+
fkdd2 | "RI_FKey_check_ins" | 5 | t | t
577+
fkdd2 | "RI_FKey_check_upd" | 17 | t | t
578+
fkdi | "RI_FKey_check_ins" | 5 | t | f
579+
fkdi | "RI_FKey_check_upd" | 17 | t | f
580+
fkdi2 | "RI_FKey_check_ins" | 5 | t | f
581+
fkdi2 | "RI_FKey_check_upd" | 17 | t | f
582+
fknd | "RI_FKey_check_ins" | 5 | f | f
583+
fknd | "RI_FKey_check_upd" | 17 | f | f
584+
fknd2 | "RI_FKey_check_ins" | 5 | f | f
585+
fknd2 | "RI_FKey_check_upd" | 17 | f | f
586+
(12 rows)
587+
528588
-- temp tables should go away by themselves, need not drop them.
529589
-- test check constraint adding
530590
create table atacc1 ( test int );

‎src/test/regress/sql/alter_table.sql

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,39 @@ ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest1, ftest2)
404404
-- As does this...
405405
ALTER TABLE FKTABLE ADD FOREIGN KEY(ftest2, ftest1)
406406
references pktable(ptest1, ptest2);
407+
DROP TABLE FKTABLE;
408+
DROP TABLE PKTABLE;
409+
410+
-- Test that ALTER CONSTRAINT updates trigger deferrability properly
411+
412+
CREATE TEMP TABLE PKTABLE (ptest1 int primary key);
413+
CREATE TEMP TABLE FKTABLE (ftest1 int);
414+
415+
ALTER TABLE FKTABLE ADD CONSTRAINT fknd FOREIGN KEY(ftest1) REFERENCES pktable
416+
ON DELETE CASCADE ON UPDATE NO ACTION NOT DEFERRABLE;
417+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdd FOREIGN KEY(ftest1) REFERENCES pktable
418+
ON DELETE CASCADE ON UPDATE NO ACTION DEFERRABLE INITIALLY DEFERRED;
419+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdi FOREIGN KEY(ftest1) REFERENCES pktable
420+
ON DELETE CASCADE ON UPDATE NO ACTION DEFERRABLE INITIALLY IMMEDIATE;
421+
422+
ALTER TABLE FKTABLE ADD CONSTRAINT fknd2 FOREIGN KEY(ftest1) REFERENCES pktable
423+
ON DELETE CASCADE ON UPDATE NO ACTION DEFERRABLE INITIALLY DEFERRED;
424+
ALTER TABLE FKTABLE ALTER CONSTRAINT fknd2 NOT DEFERRABLE;
425+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdd2 FOREIGN KEY(ftest1) REFERENCES pktable
426+
ON DELETE CASCADE ON UPDATE NO ACTION NOT DEFERRABLE;
427+
ALTER TABLE FKTABLE ALTER CONSTRAINT fkdd2 DEFERRABLE INITIALLY DEFERRED;
428+
ALTER TABLE FKTABLE ADD CONSTRAINT fkdi2 FOREIGN KEY(ftest1) REFERENCES pktable
429+
ON DELETE CASCADE ON UPDATE NO ACTION NOT DEFERRABLE;
430+
ALTER TABLE FKTABLE ALTER CONSTRAINT fkdi2 DEFERRABLE INITIALLY IMMEDIATE;
431+
432+
SELECT conname, tgfoid::regproc, tgtype, tgdeferrable, tginitdeferred
433+
FROM pg_trigger JOIN pg_constraint con ON con.oid = tgconstraint
434+
WHERE tgrelid = 'pktable'::regclass
435+
ORDER BY 1,2,3;
436+
SELECT conname, tgfoid::regproc, tgtype, tgdeferrable, tginitdeferred
437+
FROM pg_trigger JOIN pg_constraint con ON con.oid = tgconstraint
438+
WHERE tgrelid = 'fktable'::regclass
439+
ORDER BY 1,2,3;
407440

408441
-- temp tables should go away by themselves, need not drop them.
409442

0 commit comments

Comments
 (0)