Re: [BUGS] BUG #14808: V10-beta4, backend abort

Thomas Munro <thomas.munro@enterprisedb.com> writes:
> On Fri, Sep 15, 2017 at 9:45 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Attached is a draft patch for this.

> Some initial feedback:

> The effects of multiple ModifyTable nodes on the same table are
> merged.  Good.  I doubt anyone will notice but it might warrant a note
> somewhere that there is a user-visible change here: previously if you
> did this (without TTs) your trigger would have fired twice.

Check.  I've not yet looked at the documentation angle here.

> What is going on here?

Hmm ... that looks odd, but I'm too tired to debug it right now.

> Ok, that works and yeah it may be conceptually better.  Also, maybe
> the tcs_original_insert_tuple as member of TCS is a bit clunky and
> could be reconsidered later: I was trying to avoid widening a bunch of
> function calls, but that might have been optimising for the wrong
> thing...

Yeah, both tcs_map and tcs_original_insert_tuple feel like they ought
to be function parameters not persistent state.  But we can leave that
sort of refactoring for later.

> Yeah, my suggestion of a hash table was overkill.  (Maybe in future if
> we change our rules around inheritance this could finish up being
> searched for a lot of child tables; we can cross that bridge when we
> come to it.)

Exactly; I doubt it matters now, and if it ever does we can improve
the lookup infrastructure later.

> I'm a little confused about the "closed" flag.  This seems to imply
> that it's possible for there to be more than one separate tuplestore
> for a given (table, operation) in a given trigger execution context.
> Why is that OK?

The thought I had was that once we've fired any triggers that look at
a transition table, we don't want that table to change anymore; it'd
be bad if different triggers on the same event saw different tables.
So the "closed" flag is intended to shut off additional tuple additions.
If any happen (which I'm not sure if it's possible without a rogue
C-coded trigger), we'll establish a new set of transition tables for
them, and if relevant, fire AFTER STATEMENT triggers again to process
those tables.  So this sort of breaks the "one transition table and
one AFTER STATEMENT trigger firing per statement" concept, but the
alternative seems worse.  I hope that the case isn't reachable without
weird trigger behavior, though.

> I suppose that it might be possible for AfterTriggersTableData to
> record the location of a previously queued event so that you can later
> disable it and queue a replacement, with the effect of suppressing
> earlier firings rather than later ones.  That might make sense if you
> think that after statement triggers should fire after all row
> triggers.  I can't figure out from the spec whether that's expected
> and I'm not sure if it's useful.

Hm.  There could be more than one such event, but maybe we can make
something of that idea.  We could expect that the events in question
are consecutive in the event list, I think ...
        regards, tom lane


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