aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2017-09-10 14:59:56 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2017-09-10 14:59:56 -0400
commit3c435952176ae5d294b37e5963cd72ddb66edead (patch)
tree3633c94378b49189eb2806afb21bdfae25e3a6c1 /src
parent610bbdd8acfcbeedad1176188f53ce5c7905e280 (diff)
downloadpostgresql-3c435952176ae5d294b37e5963cd72ddb66edead.tar.gz
postgresql-3c435952176ae5d294b37e5963cd72ddb66edead.zip
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
Diffstat (limited to 'src')
-rw-r--r--src/backend/commands/trigger.c4
-rw-r--r--src/backend/executor/nodeModifyTable.c10
-rw-r--r--src/test/regress/expected/triggers.out52
-rw-r--r--src/test/regress/sql/triggers.sql41
4 files changed, 104 insertions, 3 deletions
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index da0850bfd6d..bbfbc06db90 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -5474,7 +5474,9 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
new_shared.ats_tgoid = trigger->tgoid;
new_shared.ats_relid = RelationGetRelid(rel);
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],
&new_event, &new_shared);
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index bd847787398..49586a3c032 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -2318,8 +2318,14 @@ ExecEndModifyTable(ModifyTableState *node)
{
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);
/*
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index ac132b042d5..2f8029a2f73 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2257,6 +2257,58 @@ create trigger my_table_multievent_trig
for each statement execute procedure dump_insert();
ERROR: Transition tables cannot be specified for triggers with more than one event
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
drop function dump_insert();
drop function dump_update();
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index b10159a1cf2..c6deb56c507 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1771,6 +1771,47 @@ create trigger my_table_multievent_trig
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
drop function dump_insert();
drop function dump_update();