Re: Remembering bug #6123

On Jan12, 2012, at 17:30 , Tom Lane wrote:
> Actually, on reflection there might be a reason for checking
> update_ctid, with a view to allowing "harmless" cases. I see
> these cases:
>
> * UPDATE finds a trigger already updated the row: must throw error
> since we can't apply the update.
>
> * UPDATE finds a trigger already deleted the row: arguably, we could
> let the deletion stand and ignore the update action.

I've argued against that in the past, and I still think it's a bad idea.
The BEFORE UPDATE trigger might have done some actions which aren't valid
now that the row has been deleted. If it actually *is* safe to let a
self-delete take precent, the BEFORE UPDATE trigger ought to check whether
the row still exists, and return NULL if it doesn't.

> * DELETE finds a trigger already updated the row: must throw error
> since we can't apply the delete.
>
> * DELETE finds a trigger already deleted the row: arguably, there's
> no reason to complain.

I'm not convinced that is a a good idea, either. If we do that, there will
essentially be two BEFORE DELETE trigger invocations for a single deleted
row, which seems wrong. And again - if a BEFORE DELETE trigger *does* deal
with this case nicely, it simply has to check whether the row still exists
before returning non-NULL.

Also, without these exceptions, the behaviour (post-patch) is simply
explain by

Either don't cause recursive same-row updates from BEFORE trigger,
or have your trigger return NULL.

and except for the case of multiple BEFORE triggers on the same table
you can always assume that

A BEFORE trigger's view of a tuple modifications always reflects the
actual modification that will eventually happen (or an error is
thrown)

Adding a lists of "buts" and "ifs" to these rules has a higher chance of
adding confusion and causing bugs than to actually help people, IMHO.

Especially since (judged from the number of years the current behaviour
as stood undisputed) these cases seems to arise quite infrequently in
practice.

best regards,
Florian Pflug