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

Reply via email to