Re: BUG #14808: V10-beta4, backend abort

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, phb07(at)apra(dot)asso(dot)fr, Andrew Gierth <rhodiumtoad(at)postgresql(dot)org>, PostgreSQL mailing lists <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: BUG #14808: V10-beta4, backend abort
Date: 2017-09-11 13:08:44
Message-ID: 14016.1505135324@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> As an idea for a v11 patch, I wonder if it would be better to count
> references instead of leaking the TCS as soon as fk-on-cascade
> triggers enter the picture.

I thought of reference counting as well, but I think it's not really
necessary if we organize things correctly. The key problem here IMO
is that the transition tables need to be attached to a trigger execution
context, rather than supposing that they can be completely managed by
ModifyTable plan nodes.

I dug around in the SQL spec and convinced myself that indeed it does
require all RI updates triggered by a single statement to be presented
in a single transition table. Concretely, it says

A trigger execution context consists of a set of state changes.
Within a trigger execution context, each state change is uniquely
identified by a trigger event, a subject table, and a column list.
The trigger event can be DELETE, INSERT, or UPDATE. A state change
SC contains a set of transitions, a set of statement-level triggers
considered as executed for SC, and a set of row-level triggers,
each paired with the set of rows in SC for which it is considered
as executed.

Note the "uniquely identified" bit --- you aren't allowed to have multiple
SCs for the same table and same kind of event, at least up to the bit
about column lists. I've not fully wrapped my head around the column list
part of it, but certainly all effects of a particular RI constraint will
have the same column list.

Now, the lifespan of a trigger execution context is one SQL-statement,
but that's one user-executed SQL-statement --- the queries that we gin
up for RI enforcement are an implementation detail that don't get their
own context. (The part of the spec that defines RI actions seems pretty
clear that the actions insert state changes into the active statement's
trigger execution context, not create their own context.)

It's also interesting in this context to re-read commit 9cb840976,
which is what created the "skip trigger" business. That exhibits a
practical problem that you hit if you don't do it like this.

So ISTM that basically we need trigger.c to own the transition tables.
ModifyTable, instead of just creating a TCS, needs to ask trigger.c for a
TCS relevant to its target table and command type (and column list if we
decide we need to bother with that). trigger.c would discard TCSes during
AfterTriggerEndQuery, where it closes out a given query_depth level.

This actually seems like it might not be that big a change, other than
the issue of what do the column lists mean exactly. Maybe we should try
to get it done for v10, rather than shipping known-non-spec-compliant
behavior.

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Alvaro Herrera 2017-09-11 13:18:41 Re: BUG #14808: V10-beta4, backend abort
Previous Message Thomas Munro 2017-09-10 21:46:52 Re: BUG #14808: V10-beta4, backend abort