Re: [HACKERS] PG10 transition tables, wCTEs and multiple operationson the same table

On Tue, Jun 6, 2017 at 12:58 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 4 June 2017 at 06:41, Kevin Grittner <kgrittn@gmail.com> wrote:
>> On Fri, Jun 2, 2017 at 8:46 PM, Thomas Munro
>> <thomas.munro@enterprisedb.com> wrote:
>>
>>> So, afterTriggers.query_stack is used to handle the reentrancy that
>>> results from triggers running further statements that might fire
>>> triggers.  It isn't used for dealing with extra ModifyTable nodes that
>>> can appear in a plan because of wCTEs.  Could it also be used for that
>>> purpose?  I think that would only work if it is the case that each
>>> ModifyTable node begin and then runs to completion (ie no interleaving
>>> of wCTE execution) and then its AS trigger fires, which I'm not sure
>>> about.
>>
>> I don't think we want to commit to anything that depends on a CTE
>> creating an optimization fence, although *maybe* that would be OK in
>> the case of DML as a CTE.  That's a pretty special case; I'm not
>> sure whether the standard discusses it.
>
> It's definitely fine to require a fence for wCTEs. They're an
> extension to the standard, and it's pretty much necessary that we not
> pull up / push down across them since we don't want to affect the
> side-effects (row changes). If there are any cases where it's safe,
> they'll take some careful thought.
>
> It's only standard CTEs (SELECT-based) that I think matter for the
> optimisation fence behaviour.

After sleeping on it, I don't think we need to make that decision here
though.  I think it's better to just move the tuplestores into
ModifyTableState so that each embedded DML statement has its own, and
have ModifyTable pass them to the trigger code explicitly.  I think
I'd like to do that via the TransitionCaptureState object that I
proposed elsewhere, but I'll hold off on doing anything until I hear
from interested committers on which way we're going here, time being
short.

Call me an anti-globalisation (of variables) protestor.

-- 
Thomas Munro
http://www.enterprisedb.com