Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 445035a

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 99f13d1 commit 445035a

File tree

3 files changed

+119
-5
lines changed

3 files changed

+119
-5
lines changed

‎src/backend/commands/tablecmds.c

+26-5
Original file line numberDiff line numberDiff line change
@@ -6740,16 +6740,34 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
67406740

67416741
while (HeapTupleIsValid(tgtuple = systable_getnext(tgscan)))
67426742
{
6743+
Form_pg_trigger tgform = (Form_pg_trigger) GETSTRUCT(tgtuple);
67436744
Form_pg_trigger copy_tg;
67446745

6746+
/*
6747+
* Remember OIDs of other relation(s) involved in FK constraint.
6748+
* (Note: it's likely that we could skip forcing a relcache inval
6749+
* for other rels that don't have a trigger whose properties
6750+
* change, but let's be conservative.)
6751+
*/
6752+
if (tgform->tgrelid != RelationGetRelid(rel))
6753+
otherrelids = list_append_unique_oid(otherrelids,
6754+
tgform->tgrelid);
6755+
6756+
/*
6757+
* Update deferrability of RI_FKey_noaction_del,
6758+
* RI_FKey_noaction_upd, RI_FKey_check_ins and RI_FKey_check_upd
6759+
* triggers, but not others; see createForeignKeyTriggers and
6760+
* CreateFKCheckTrigger.
6761+
*/
6762+
if (tgform->tgfoid != F_RI_FKEY_NOACTION_DEL &&
6763+
tgform->tgfoid != F_RI_FKEY_NOACTION_UPD &&
6764+
tgform->tgfoid != F_RI_FKEY_CHECK_INS &&
6765+
tgform->tgfoid != F_RI_FKEY_CHECK_UPD)
6766+
continue;
6767+
67456768
copyTuple = heap_copytuple(tgtuple);
67466769
copy_tg = (Form_pg_trigger) GETSTRUCT(copyTuple);
67476770

6748-
/* Remember OIDs of other relation(s) involved in FK constraint */
6749-
if (copy_tg->tgrelid != RelationGetRelid(rel))
6750-
otherrelids = list_append_unique_oid(otherrelids,
6751-
copy_tg->tgrelid);
6752-
67536771
copy_tg->tgdeferrable = cmdcon->deferrable;
67546772
copy_tg->tginitdeferred = cmdcon->initdeferred;
67556773
simple_heap_update(tgrel, &copyTuple->t_self, copyTuple);
@@ -7511,6 +7529,9 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint,
75117529

75127530
/*
75137531
* Create the triggers that implement an FK constraint.
7532+
*
7533+
* NB: if you change any trigger properties here, see also
7534+
* ATExecAlterConstraint.
75147535
*/
75157536
static void
75167537
createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,

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

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

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

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

411444
-- temp tables should go away by themselves, need not drop them.
412445

0 commit comments

Comments
 (0)