Re: BUG #14808: V10-beta4, backend abort
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Michael Paquier <michael.paquier@gmail.com>, phb07@apra.asso.fr,
Andrew Gierth <rhodiumtoad@postgresql.org>,
PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Date: 2017-09-09T18:59:54Z
Lists: pgsql-bugs
Attachments
- trigger-fix-quick-hack.patch (text/x-diff) patch
I wrote: > What is happening is that the AFTER triggers are queuing more triggers, > which have TransitionCaptureStates associated, but > ExecEndModifyTable thinks it can DestroyTransitionCaptureState > unconditionally. When the subsidiary triggers eventually get executed, > their ats_transition_capture pointers are pointing at garbage. On closer inspection, the issue is specific to TCS-using AFTER triggers that are being fired as a result of foreign key enforcement triggers. In the example, each row deleted from myTbl1 causes firing of the ON DELETE CASCADE enforcement trigger, which will execute a DELETE against myTbl3. That won't delete anything (since myTbl3 is empty) but we nonetheless queue a firing of the TCS-using AFTER STATEMENT trigger for myTbl3. 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. I do not recall the exact details of why that is, but I believe the intention is to fire RI-triggered triggers at the end of the outer statement rather than exposing the individual RI action as a statement. I made a quick hack to not delete the TCS if EXEC_FLAG_SKIP_TRIGGERS is set, as attached. The example succeeds given this. However, note that the AFTER STATEMENT trigger will be fired once per myTbl1 row deletion, each time seeing a transition table consisting of only the myTbl3 rows that went away as a result of that one single row deletion. I'm not sure that this is per the letter or spirit of the SQL spec. If it isn't, I don't think there is much we can do about it for v10. In v11 or later, we could think about somehow merging the transition tables for all the RI actions triggered by one statement. This might well need to go along with rewriting the RI framework to use statement triggers and TCS state itself. I think people had already muttered about doing that. > The other question that seems pretty relevant is why the subsidiary > triggers, which are the constraint triggers associated with the > tables' foreign keys, are getting queued with TransitionCaptureState > pointers in the first place. This seems horridly expensive and > unnecessary. It also directly contradicts the claim in > MakeTransitionCaptureState that constraint triggers cannot have > transition tables. The other part of the attached patch tweaks AfterTriggerSaveEvent to not store an ats_transition_capture pointer for a deferrable trigger event. This doesn't have anything directly to do with the current bug, because although the RI triggers are being stored with such pointers, they aren't actually dereferencing them. However, it seems like a good idea anyway, to (1) ensure that we don't have dangling pointers in the trigger queue, and (2) possibly allow more merging of shared trigger states. regards, tom lane
Commits
-
Fix SQL-spec incompatibilities in new transition table feature.
- 54d4d0ff6cd4 10.0 landed
- 0f79440fb0b4 11.0 landed
-
Quick-hack fix for foreign key cascade vs triggers with transition tables.
- 5c11717185bc 10.0 landed
- 3c435952176a 11.0 landed
-
Fix transition tables for ON CONFLICT.
- 8c55244ae379 10.0 cited
-
Fix transition tables for wCTEs.
- c46c0e5202e8 10.0 cited
-
Repair problems occurring when multiple RI updates have to be done to the same
- 9cb84097623e 8.3.0 cited