Обсуждение: Foreign key trigger timing bug?
I had an open 8.1 item that was: o fix foreign trigger timing issue Would someone supply text for a TODO entry on this, as I don't think we fixed it in 8.1. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
On Wednesday 07 December 2005 09:33, Bruce Momjian wrote: > I had an open 8.1 item that was: > > o fix foreign trigger timing issue Stephan Szabo had this to say to me when I was asking him about his progress on this issue a while back. There are some fundamental issues right now between before triggers and foreign keys based on how we act upon rows for the same statement that have been modified in the before trigger (which is to say that the outer statement does not act upon them). > > Would someone supply text for a TODO entry on this, as I don't think we > fixed it in 8.1. No it's not yet resolved. -- Darcy Buskermolen Wavefire Technologies Corp. http://www.wavefire.com ph: 250.717.0200 fx: 250.763.1759
On Wed, 7 Dec 2005, Bruce Momjian wrote: > I had an open 8.1 item that was: > > o fix foreign trigger timing issue > > Would someone supply text for a TODO entry on this, as I don't think we > fixed it in 8.1. I'd split this into two separate items now. Fix before delete triggers on cascaded deletes to run after the cascaded delete is done. This is odd, but seems to be what the spec requires. Fix problems with referential action caused before triggers that modify rows that would also be modified by the referential action. Right now, this has a few symptoms, either you can get spurious seeming errors from the constraint or you can end up with invalid data in the referencing table. As far as I can see, the spec doesn't have much to say about this because the spec doesn't seem to allow before triggers to modify tables.
On 12/7/2005 4:50 PM, Stephan Szabo wrote: > On Wed, 7 Dec 2005, Bruce Momjian wrote: > >> I had an open 8.1 item that was: >> >> o fix foreign trigger timing issue >> >> Would someone supply text for a TODO entry on this, as I don't think we >> fixed it in 8.1. > > I'd split this into two separate items now. > > Fix before delete triggers on cascaded deletes to run after the cascaded > delete is done. This is odd, but seems to be what the spec requires. Ugh, that sounds ugly. One problem I see is, what do we do if the BEFORE trigger then returns NULL (to skip the delete). The cascaded operations are already done. Do we have to execute the cascaded deletes in a subtransaction or do we disallow the skip in this case? Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
On Thu, 8 Dec 2005, Jan Wieck wrote: > On 12/7/2005 4:50 PM, Stephan Szabo wrote: > > > On Wed, 7 Dec 2005, Bruce Momjian wrote: > > > >> I had an open 8.1 item that was: > >> > >> o fix foreign trigger timing issue > >> > >> Would someone supply text for a TODO entry on this, as I don't think we > >> fixed it in 8.1. > > > > I'd split this into two separate items now. > > > > Fix before delete triggers on cascaded deletes to run after the cascaded > > delete is done. This is odd, but seems to be what the spec requires. > > Ugh, that sounds ugly. Yeah. I really don't understand it, but it appears to me to be explicitly different in the spec for on delete cascade even compared to the rest of the referential actions. > One problem I see is, what do we do if the BEFORE > trigger then returns NULL (to skip the delete). The cascaded operations > are already done. Do we have to execute the cascaded deletes in a > subtransaction or do we disallow the skip in this case? I think we'd have disallow skipping. Especially since skipping would probably end up with a violated constraint.
Stephan Szabo <sszabo@megazone.bigpanda.com> writes:
> Yeah. I really don't understand it, but it appears to me to be explicitly
> different in the spec for on delete cascade even compared to the rest of
> the referential actions.
>> One problem I see is, what do we do if the BEFORE
>> trigger then returns NULL (to skip the delete). The cascaded operations
>> are already done. Do we have to execute the cascaded deletes in a
>> subtransaction or do we disallow the skip in this case?
> I think we'd have disallow skipping. Especially since skipping would
> probably end up with a violated constraint.
That seems to me to be a sufficient reason to not follow the spec in
this respect. A BEFORE trigger should be run BEFORE anything happens,
full stop. I can't think of any good reason why the spec's semantics
are better. (It's not like our triggers are exactly spec-compatible
anyway.)
regards, tom lane
On 12/8/2005 8:53 PM, Tom Lane wrote: > Stephan Szabo <sszabo@megazone.bigpanda.com> writes: >> Yeah. I really don't understand it, but it appears to me to be explicitly >> different in the spec for on delete cascade even compared to the rest of >> the referential actions. > >>> One problem I see is, what do we do if the BEFORE >>> trigger then returns NULL (to skip the delete). The cascaded operations >>> are already done. Do we have to execute the cascaded deletes in a >>> subtransaction or do we disallow the skip in this case? > >> I think we'd have disallow skipping. Especially since skipping would >> probably end up with a violated constraint. > > That seems to me to be a sufficient reason to not follow the spec in > this respect. A BEFORE trigger should be run BEFORE anything happens, > full stop. I can't think of any good reason why the spec's semantics > are better. (It's not like our triggers are exactly spec-compatible > anyway.) It doesn't lead to a violated constraint. bar references foo on delete cascade, now delete from foo will first delete from bar, then the before trigger on foo skips the delete. And besides, as the other post (Trigger preventing delete causes circumvention of FK) in GENERAL shows, triggers can break RI anyway. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Stephan Szabo wrote: > On Wed, 7 Dec 2005, Bruce Momjian wrote: > > > I had an open 8.1 item that was: > > > > o fix foreign trigger timing issue > > > > Would someone supply text for a TODO entry on this, as I don't think we > > fixed it in 8.1. > > I'd split this into two separate items now. > > Fix before delete triggers on cascaded deletes to run after the cascaded > delete is done. This is odd, but seems to be what the spec requires. > > Fix problems with referential action caused before triggers that modify > rows that would also be modified by the referential action. Right now, > this has a few symptoms, either you can get spurious seeming errors from > the constraint or you can end up with invalid data in the referencing > table. As far as I can see, the spec doesn't have much to say about this > because the spec doesn't seem to allow before triggers to modify tables. I have updated the CREATE TRIGGER documentation to highlight these items, and the fact we consider our current behavior on the first item to be correct. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/create_trigger.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/ref/create_trigger.sgml,v retrieving revision 1.42 diff -c -c -r1.42 create_trigger.sgml *** doc/src/sgml/ref/create_trigger.sgml 1 Nov 2005 21:09:50 -0000 1.42 --- doc/src/sgml/ref/create_trigger.sgml 9 Dec 2005 19:38:27 -0000 *************** *** 241,253 **** function that executes the desired commands. </para> </listitem> </itemizedlist> </para> <para> SQL specifies that multiple triggers should be fired in time-of-creation order. <productname>PostgreSQL</productname> uses ! name order, which was judged more convenient to work with. </para> <para> --- 241,265 ---- function that executes the desired commands. </para> </listitem> + </itemizedlist> </para> <para> SQL specifies that multiple triggers should be fired in time-of-creation order. <productname>PostgreSQL</productname> uses ! name order, which was judged to be more convenient. ! </para> ! ! <para> ! SQL specifies that <literal>BEFORE DELETE</literal> triggers on cascaded ! deletes fire <emphasis>after</> the cascaded <literal>DELETE</> completes. ! The <productname>PostgreSQL</productname> behavior is for <literal>BEFORE ! DELETE</literal> to always fire before the delete action, even a cascading ! one. This is considered more consistent. There is also unpredictable ! behavior when <literal>BEFORE</literal> triggers modify rows that are later ! to be modified by referential actions. This can lead to contraint violations ! or stored data that does not honor the referential constraint. </para> <para>
On Fri, 9 Dec 2005, Jan Wieck wrote: > On 12/8/2005 8:53 PM, Tom Lane wrote: > > > Stephan Szabo <sszabo@megazone.bigpanda.com> writes: > >> Yeah. I really don't understand it, but it appears to me to be explicitly > >> different in the spec for on delete cascade even compared to the rest of > >> the referential actions. > > > >>> One problem I see is, what do we do if the BEFORE > >>> trigger then returns NULL (to skip the delete). The cascaded operations > >>> are already done. Do we have to execute the cascaded deletes in a > >>> subtransaction or do we disallow the skip in this case? > > > >> I think we'd have disallow skipping. Especially since skipping would > >> probably end up with a violated constraint. > > > > That seems to me to be a sufficient reason to not follow the spec in > > this respect. A BEFORE trigger should be run BEFORE anything happens, > > full stop. I can't think of any good reason why the spec's semantics > > are better. (It's not like our triggers are exactly spec-compatible > > anyway.) > > It doesn't lead to a violated constraint. bar references foo on delete > cascade, now delete from foo will first delete from bar, then the before > trigger on foo skips the delete. That's not the right case I think. Pseudo example: create table a create table b references a on delete cascade create before trigger on b that sometimes skips a delete to b insert into a and b. delete from a -- AFAICS, you can end up with a row in b that no longer has its associated row in a since the a row will be deleted but there's no guarantee its referencing rows in b will have successfully been deleted. > And besides, as the other post (Trigger preventing delete causes > circumvention of FK) in GENERAL shows, triggers can break RI anyway. Yeah, although fixing the cases where the trigger interacted badly with before triggers was the point of the posts that started this. The original problem was with a case where it acted differently, but it's fairly related.
On 12/9/2005 8:27 PM, Stephan Szabo wrote: > On Fri, 9 Dec 2005, Jan Wieck wrote: > >> On 12/8/2005 8:53 PM, Tom Lane wrote: >> >> > Stephan Szabo <sszabo@megazone.bigpanda.com> writes: >> >> Yeah. I really don't understand it, but it appears to me to be explicitly >> >> different in the spec for on delete cascade even compared to the rest of >> >> the referential actions. >> > >> >>> One problem I see is, what do we do if the BEFORE >> >>> trigger then returns NULL (to skip the delete). The cascaded operations >> >>> are already done. Do we have to execute the cascaded deletes in a >> >>> subtransaction or do we disallow the skip in this case? >> > >> >> I think we'd have disallow skipping. Especially since skipping would >> >> probably end up with a violated constraint. >> > >> > That seems to me to be a sufficient reason to not follow the spec in >> > this respect. A BEFORE trigger should be run BEFORE anything happens, >> > full stop. I can't think of any good reason why the spec's semantics >> > are better. (It's not like our triggers are exactly spec-compatible >> > anyway.) >> >> It doesn't lead to a violated constraint. bar references foo on delete >> cascade, now delete from foo will first delete from bar, then the before >> trigger on foo skips the delete. > > That's not the right case I think. > > Pseudo example: > > create table a > create table b references a on delete cascade > create before trigger on b that sometimes skips a delete to b > insert into a and b. > delete from a > > -- AFAICS, you can end up with a row in b that no longer has its > associated row in a since the a row will be deleted but there's no > guarantee its referencing rows in b will have successfully been deleted. Yes, you can deliberately break referential integrity with that. But you know what? I think the overall waste of performance and developer time, required to "fix" this rather exotic (and idiotic) problem, is too high to seriously consider it. Jan > >> And besides, as the other post (Trigger preventing delete causes >> circumvention of FK) in GENERAL shows, triggers can break RI anyway. > > Yeah, although fixing the cases where the trigger interacted badly with > before triggers was the point of the posts that started this. The original > problem was with a case where it acted differently, but it's fairly > related. > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: explain analyze is your friend -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
> On 12/9/2005 8:27 PM, Stephan Szabo wrote: > > On Fri, 9 Dec 2005, Jan Wieck wrote: > > > >> On 12/8/2005 8:53 PM, Tom Lane wrote: > >> > >> > Stephan Szabo <sszabo@megazone.bigpanda.com> writes: > >> >> Yeah. I really don't understand it, but it appears to me to be explicitly > >> >> different in the spec for on delete cascade even compared to the rest of > >> >> the referential actions. > >> > > >> >>> One problem I see is, what do we do if the BEFORE > >> >>> trigger then returns NULL (to skip the delete). The cascaded operations > >> >>> are already done. Do we have to execute the cascaded deletes in a > >> >>> subtransaction or do we disallow the skip in this case? > >> > > >> >> I think we'd have disallow skipping. Especially since skipping would > >> >> probably end up with a violated constraint. > >> > > >> > That seems to me to be a sufficient reason to not follow the spec in > >> > this respect. A BEFORE trigger should be run BEFORE anything happens, > >> > full stop. I can't think of any good reason why the spec's semantics > >> > are better. (It's not like our triggers are exactly spec-compatible > >> > anyway.) > >> > >> It doesn't lead to a violated constraint. bar references foo on delete > >> cascade, now delete from foo will first delete from bar, then the before > >> trigger on foo skips the delete. > > > > That's not the right case I think. > > > > Pseudo example: > > > > create table a > > create table b references a on delete cascade > > create before trigger on b that sometimes skips a delete to b > > insert into a and b. > > delete from a > > > > -- AFAICS, you can end up with a row in b that no longer has its > > associated row in a since the a row will be deleted but there's no > > guarantee its referencing rows in b will have successfully been deleted. > > Yes, you can deliberately break referential integrity with that. But you > know what? I think the overall waste of performance and developer time, > required to "fix" this rather exotic (and idiotic) problem, is too high > to seriously consider it. Well, the case that brought up the original question was one where the before trigger updated rows that were going to be affected by the cascaded delete. Before this worked by accident, now it gives an error (even though the key wasn't changed due to some other possibilities of violation forcing the check). The problem is that if we're not consistent about what violation cases are acceptable, it's hard to diagnose if something is an actual bug or merely an acceptable side effect. :)