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 transaction
b) Some items were already marked by a parent transaction
c) Some items were already marked 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.