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

On Tue, Sep 12, 2017 at 1:22 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Tue, Sep 12, 2017 at 12:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> OTOH, looking at the text of
>> the spec, I think it's darn hard to justify the behavior shown above.
>
> Yeah.  I assume we always fired statement triggers for each separate
> instance of the same table mentioned in a wCTE since they were
> invented.  I just confirmed that that is the case in 9.6.  That may
> not have been in the spirit of the spec, but it's hard to say because
> the spec doesn't have wCTEs IIUC and it mattered less because they
> didn't receive any data.
>
> Now that they can optionally see data resulting from modifications, it
> seems pretty hard to use this feature to build anything that consumes
> the transition data and has to be reliable (matview state,
> replication-like systems etc) if we make any choice other than (1)
> each instance of a given table fires a statement trigger separately
> and sees only the rows it touched, or (2) the statement trigger is
> fired once for all instances of a table, and sees all the transition
> tuples.  Based on the SQL spec excerpts you've highlighted, I suppose
> it seems likely that if the spec had wCTEs it would expect them to
> work like (2).

So I guess there are about 3 parts to this puzzle:

1.  Merging the transition tables when there are multiple wCTEs
referencing the same table.  Here's one idea:  Rename
MakeTransitionCaptureState() to GetTransitionCaptureState() and use a
hash table keyed by table OID in
afterTriggers.transition_capture_states[afterTriggers.query_depth] to
find the TCS for the given TriggerDesc or create it if not found, so
that all wCTEs find the same TransitionCaptureState object.  The all
existing callers continue to do what they're doing now, but they'll be
sharing TCSs appropriately with other things in the plan.  Note that
TransitionCaptureState already holds tuplestores for each operation
(INSERT, UPDATE, DELETE) so the OID of the table alone is a suitable
key for the hash table (assuming we are ignoring the column-list part
of the spec as you suggested).

2.  Hiding the fact that we implement fk CASCADE using another level
of queries.   Perhaps we could arrange for
afterTriggers.transition_capture_states[afterTriggers.query_depth] to
point to the same hash table as query_depth - 1, so that the effects
of statements at this implementation-internal level appear to the user
as part of the the level below?

3.  Merging the invocation after statement firing so that if you
updated the same table directly and also via a wCTE and also
indirectly via fk ON DELETE/UPDATE trigger then you still only get one
invocation of the after statement trigger.  Not sure exactly how...
perhaps using a flag in the TransitionCaptureState to prevent multiple
firings.  As I argued in the above-quoted email, if we've merged the
transition tables then we'll need to merge the trigger firing too or
it won't be possible to make coherent integrity, summary, replication
etc systems using TT triggers (even though that's a user-visible
change in after statement firing behaviour for wCTEs compared to
earlier releases).

Does this make any sense?

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


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