[COMMITTERS] pgsql: Quick-hack fix for foreign key cascade vs triggers withtransiti - Mailing list pgsql-committers

From Tom Lane
Subject [COMMITTERS] pgsql: Quick-hack fix for foreign key cascade vs triggers withtransiti
Date
Msg-id E1dr7TJ-000544-0J@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
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

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/3c435952176ae5d294b37e5963cd72ddb66edead

Modified Files
--------------
src/backend/commands/trigger.c         |  4 ++-
src/backend/executor/nodeModifyTable.c | 10 +++++--
src/test/regress/expected/triggers.out | 52 ++++++++++++++++++++++++++++++++++
src/test/regress/sql/triggers.sql      | 41 +++++++++++++++++++++++++++
4 files changed, 104 insertions(+), 3 deletions(-)


--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: [COMMITTERS] pgsql: Add a test harness for the red-black tree code.
Next
From: Peter Eisentraut
Date:
Subject: [COMMITTERS] pgsql: Message style fixes