On Fri, Jun 9, 2017 at 8:41 AM, Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
>>>>>> "Thomas" == Thomas Munro <thomas.munro@enterprisedb.com> writes:
>
> Thomas> So, afterTriggers.query_stack is used to handle the reentrancy
> Thomas> that results from triggers running further statements that
> Thomas> might fire triggers. It isn't used for dealing with extra
> Thomas> ModifyTable nodes that can appear in a plan because of wCTEs.
> Thomas> Could it also be used for that purpose? I think that would
> Thomas> only work if it is the case that each ModifyTable node begin
> Thomas> and then runs to completion (ie no interleaving of wCTE
> Thomas> execution) and then its AS trigger fires, which I'm not sure
> Thomas> about.
>
> There's a problem with this which I didn't see anyone mention (though
> I've not read the whole history); existing users of wCTEs rely on the
> fact that no AFTER triggers are run until _all_ modifications are
> completed. If you change that, then you can't use wCTEs to insert into
> tables with FK checks without knowing the order in which the individual
> wCTEs are executed, which is currently a bit vague and non-obvious
> (which doesn't cause issues at the moment only because nothing actually
> depends on the order).
>
> for example:
> create table foo (id integer primary key);
> create table foo_bar (foo_id integer references foo, bar_id integer);
> with i1 as (insert into foo values (1))
> insert into foo_bar values (1,2),(1,3);
>
> This would fail the FK check if each insert did its own trigger firing,
> since the foo_bar insert is actually run _first_.
>
> Firing triggers earlier than they currently are would thus break
> existing working queries.
Thanks, and I agree. Later I came to the conclusion that we should
neither change nor depend on the order of execution, but in order to
avoid doing those things we simply couldn't keep using query_stack as
working space for the tuplestores -- the access pattern does not fit
stack semantics. That's why I proposed having the ModifyTable nodes
own their own transition tuplestores, and passing them explicitly to
the sites where they're needed (various trigger.c routines), instead
of accessing them via that global stack. That's the approach of the
WIP patch I posted here:
https://www.postgresql.org/message-id/CAEepm%3D1K7F08QPu%2BkGcNQ-bCcYBa3QX%3D9AeE%3Dj0doCmgqVs4Tg%40mail.gmail.com
--
Thomas Munro
http://www.enterprisedb.com