I can't believe that the coding at trigger.c line 2010 ff is correct: /* * Skip executing cancelled events, and events done by * transactions that are not aborted. */ if (!(event->dte_event & TRIGGER_DEFERRED_CANCELED) || (event->dte_event & TRIGGER_DEFERRED_DONE && TransactionIdIsValid(event->dte_done_xid) && !TransactionIdDidAbort(event->dte_done_xid))) {
Surely the sense of this is backwards, and it should be if (!(event->dte_event & TRIGGER_DEFERRED_CANCELED) && !(event->dte_event & TRIGGER_DEFERRED_DONE && TransactionIdIsValid(event->dte_done_xid) && !TransactionIdDidAbort(event->dte_done_xid))) { AFAICT we don't actually use TRIGGER_DEFERRED_CANCELED, so the existing coding never "skips" at all here, which makes it just a performance loss rather than visible misbehavior. I'm also concerned about the fact that the per-item states have dti_done_xid values distinct from the whole-event value. It's not obvious that a rollback of the subxact that did one item implies a rollback of the subxact that last marked the event as scanned. Can anyone offer a proof that that's OK? If it is OK, is it really necessary to have per-item dti_done_xid at all? Finally, surely the "Mark the event done" case should advance prev_event? As-is the code is capable of messing up the list links. regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])