Commit 3c435952 authored by Tom Lane's avatar Tom Lane

Quick-hack fix for foreign key cascade vs triggers with transition tables.

AFTER triggers using transition tables crashed if they were fired due
to a foreign key ON CASCADE update.  This is because ExecEndModifyTable
flushes the transition tables, on the assumption that any trigger that
could need them was already fired during ExecutorFinish.  Normally
that's true, because we don't allow transition-table-using triggers
to be deferred.  However, foreign key CASCADE updates force any
triggers on the referencing table to be deferred to the outer query
level, by means of the EXEC_FLAG_SKIP_TRIGGERS flag.  I don't recall
all the details of why it's like that and am pretty loath to redesign
it right now.  Instead, just teach ExecEndModifyTable to skip destroying
the TransitionCaptureState when that flag is set.  This will allow the
transition table data to survive until end of the current subtransaction.

This isn't a terribly satisfactory solution, because (1) we might be
leaking the transition tables for much longer than really necessary,
and (2) as things stand, an AFTER STATEMENT trigger will fire once per
RI updating query, ie once per row updated or deleted in the referenced
table.  I suspect that is not per SQL spec.  But redesigning this is a
research project that we're certainly not going to get done for v10.
So let's go with this hackish answer for now.

In passing, tweak AfterTriggerSaveEvent to not save the transition_capture
pointer into the event record for a deferrable trigger.  This is not
necessary to fix the current bug, but it avoids letting dangling pointers
to long-gone transition tables persist in the trigger event queue.  That's
at least a safety feature.  It might also allow merging shared trigger
states in more cases than before.

I added a regression test that demonstrates the crash on unpatched code,
and also exposes the behavior of firing the AFTER STATEMENT triggers
once per row update.

Per bug #14808 from Philippe Beaudoin.  Back-patch to v10.

Discussion: https://postgr.es/m/20170909064853.25630.12825@wrigleys.postgresql.org
parent 610bbdd8
...@@ -5474,7 +5474,9 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, ...@@ -5474,7 +5474,9 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
new_shared.ats_tgoid = trigger->tgoid; new_shared.ats_tgoid = trigger->tgoid;
new_shared.ats_relid = RelationGetRelid(rel); new_shared.ats_relid = RelationGetRelid(rel);
new_shared.ats_firing_id = 0; new_shared.ats_firing_id = 0;
new_shared.ats_transition_capture = transition_capture; /* deferrable triggers cannot access transition data */
new_shared.ats_transition_capture =
trigger->tgdeferrable ? NULL : transition_capture;
afterTriggerAddEvent(&afterTriggers.query_stack[afterTriggers.query_depth], afterTriggerAddEvent(&afterTriggers.query_stack[afterTriggers.query_depth],
&new_event, &new_shared); &new_event, &new_shared);
......
...@@ -2318,8 +2318,14 @@ ExecEndModifyTable(ModifyTableState *node) ...@@ -2318,8 +2318,14 @@ ExecEndModifyTable(ModifyTableState *node)
{ {
int i; int i;
/* Free transition tables */ /*
if (node->mt_transition_capture != NULL) * Free transition tables, unless this query is being run in
* EXEC_FLAG_SKIP_TRIGGERS mode, which means that it may have queued AFTER
* triggers that won't be run till later. In that case we'll just leak
* the transition tables till end of (sub)transaction.
*/
if (node->mt_transition_capture != NULL &&
!(node->ps.state->es_top_eflags & EXEC_FLAG_SKIP_TRIGGERS))
DestroyTransitionCaptureState(node->mt_transition_capture); DestroyTransitionCaptureState(node->mt_transition_capture);
/* /*
......
...@@ -2257,6 +2257,58 @@ create trigger my_table_multievent_trig ...@@ -2257,6 +2257,58 @@ create trigger my_table_multievent_trig
for each statement execute procedure dump_insert(); for each statement execute procedure dump_insert();
ERROR: Transition tables cannot be specified for triggers with more than one event ERROR: Transition tables cannot be specified for triggers with more than one event
drop table my_table; drop table my_table;
--
-- Test firing of triggers with transition tables by foreign key cascades
--
create table refd_table (a int primary key, b text);
create table trig_table (a int, b text,
foreign key (a) references refd_table on update cascade on delete cascade
);
create trigger trig_table_insert_trig
after insert on trig_table referencing new table as new_table
for each statement execute procedure dump_insert();
create trigger trig_table_update_trig
after update on trig_table referencing old table as old_table new table as new_table
for each statement execute procedure dump_update();
create trigger trig_table_delete_trig
after delete on trig_table referencing old table as old_table
for each statement execute procedure dump_delete();
insert into refd_table values
(1, 'one'),
(2, 'two'),
(3, 'three');
insert into trig_table values
(1, 'one a'),
(1, 'one b'),
(2, 'two a'),
(2, 'two b'),
(3, 'three a'),
(3, 'three b');
NOTICE: trigger = trig_table_insert_trig, new table = (1,"one a"), (1,"one b"), (2,"two a"), (2,"two b"), (3,"three a"), (3,"three b")
update refd_table set a = 11 where b = 'one';
NOTICE: trigger = trig_table_update_trig, old table = (1,"one a"), (1,"one b"), new table = (11,"one a"), (11,"one b")
select * from trig_table;
a | b
----+---------
2 | two a
2 | two b
3 | three a
3 | three b
11 | one a
11 | one b
(6 rows)
delete from refd_table where length(b) = 3;
NOTICE: trigger = trig_table_delete_trig, old table = (2,"two a"), (2,"two b")
NOTICE: trigger = trig_table_delete_trig, old table = (11,"one a"), (11,"one b")
select * from trig_table;
a | b
---+---------
3 | three a
3 | three b
(2 rows)
drop table refd_table, trig_table;
-- cleanup -- cleanup
drop function dump_insert(); drop function dump_insert();
drop function dump_update(); drop function dump_update();
......
...@@ -1771,6 +1771,47 @@ create trigger my_table_multievent_trig ...@@ -1771,6 +1771,47 @@ create trigger my_table_multievent_trig
drop table my_table; drop table my_table;
--
-- Test firing of triggers with transition tables by foreign key cascades
--
create table refd_table (a int primary key, b text);
create table trig_table (a int, b text,
foreign key (a) references refd_table on update cascade on delete cascade
);
create trigger trig_table_insert_trig
after insert on trig_table referencing new table as new_table
for each statement execute procedure dump_insert();
create trigger trig_table_update_trig
after update on trig_table referencing old table as old_table new table as new_table
for each statement execute procedure dump_update();
create trigger trig_table_delete_trig
after delete on trig_table referencing old table as old_table
for each statement execute procedure dump_delete();
insert into refd_table values
(1, 'one'),
(2, 'two'),
(3, 'three');
insert into trig_table values
(1, 'one a'),
(1, 'one b'),
(2, 'two a'),
(2, 'two b'),
(3, 'three a'),
(3, 'three b');
update refd_table set a = 11 where b = 'one';
select * from trig_table;
delete from refd_table where length(b) = 3;
select * from trig_table;
drop table refd_table, trig_table;
-- cleanup -- cleanup
drop function dump_insert(); drop function dump_insert();
drop function dump_update(); drop function dump_update();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment