Re: [BUGS] BUG #14808: V10-beta4, backend abort - Mailing list pgsql-bugs

From Thomas Munro
Subject Re: [BUGS] BUG #14808: V10-beta4, backend abort
Date
Msg-id CAEepm=0rNhRe8Zjf46H__au+KrAYktbefpNyFHVqn9ZfFyKV_Q@mail.gmail.com
Whole thread Raw
In response to Re: [BUGS] BUG #14808: V10-beta4, backend abort  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [BUGS] BUG #14808: V10-beta4, backend abort
List pgsql-bugs
On Mon, Sep 11, 2017 at 7:02 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>> Now, the ExecEndModifyTable instance for the DELETE supposes that
>> all TCS-using triggers must have been fired during ExecutorFinish;
>> but *that doesn't happen if EXEC_FLAG_SKIP_TRIGGERS is set*, which
>> it is because ri_PerformCheck tells SPI not to fire triggers.
>
> In view of the fact that 10rc1 wraps tomorrow, there doesn't seem
> to be time to do anything better about this than my quick hack.
> So I went ahead and pushed that, with a regression test case.

Thank you for the testing and report Philippe, and for the analysis and fix Tom.

@@ -2318,8 +2318,14 @@ ExecEndModifyTable(ModifyTableState *node){       int                     i;

-       /* Free transition tables */
-       if (node->mt_transition_capture != NULL)
+       /*
+        * Free transition tables, unless this query is being run in
+        * EXEC_FLAG_SKIP_TRIGGERS mode, which means that it may have
queued AFTER
+        * triggers that won't be run till later.  In that case we'll just leak
+        * the transition tables till end of (sub)transaction.
+        */
+       if (node->mt_transition_capture != NULL &&
+               !(node->ps.state->es_top_eflags & EXEC_FLAG_SKIP_TRIGGERS))
DestroyTransitionCaptureState(node->mt_transition_capture);

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.  The ModifyTable node would release its
reference in ExecEndModifyTable(), and the queued events' references
would be counted too.  I briefly considered a scheme like that before
proposing 501ed02c, but concluded, as it turns out incorrectly, that
there was no way for a trigger referencing node->mt_transition_capture
to fire after that point.

-- 
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

pgsql-bugs by date:

Previous
From: Matthew Maurer
Date:
Subject: Re: [BUGS] BUG #14809: Heap Corruption with deeply nested triggers.
Next
From: Tom Lane
Date:
Subject: Re: [BUGS] BUG #14808: V10-beta4, backend abort