Re: Breakage in trigger.c

On Mon, 6 Sep 2004, Tom Lane wrote:

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

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

I don't think that case can occur.

The transaction marking the event will have seen one of the following
states I believe:
a) All items were marked by this transactionb) Some items were already marked by a parent transactionc) Some items were
alreadymarked by a previous committed subtransaction
 

In the first case, it's marked them so it's okay. In the second and third,
I think the only way for the the item marking subtransaction to abort
after marking the event would involve aborting a common parent transaction
which would abort both.

I think the per-item one is necessary for SET CONSTRAINTS (some of the
deferred actions on a particular event may have been done as per b or c
above).

> Finally, surely the "Mark the event done" case should advance
> prev_event?  As-is the code is capable of messing up the list links.

As something for the future, it looks to me like subtransactions won't
delink items ever right now, where I think it'd be safe to do so for items
generated from the same subtransaction but I haven't looked to see if
we're keeping that info.