Re: Remembering bug #6123

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane wrote:
>> While that sounds relatively safe, if possibly performance-
>> impacting, it's not apparent to me how it fixes the problem you
>> complained of. The triggers you were using were modifying rows
>> other than the one being targeted by the triggering action, so a
>> test like the above would not notice that they'd done anything.

> My initial use-case was that a BEFORE DELETE trigger was deleting
> related "child" rows, and the BEFORE DELETE trigger at the child
> level was updating counts on the original (parent) row. The proposed
> change would cause an error to be thrown when the parent level
> returned a non-NULL value from its BEFORE DELETE trigger.

Ah, I see.

I suggest that the current behavior was designed for the case of
independent concurrent updates, and you have not made a good argument
for changing that. What does make sense to me, in light of these
examples, is to complain if a BEFORE trigger modifies the row "itself"
(including indirect updates). IOW, at conclusion of trigger firing
(I see no need to do it for each trigger), check to see if the target
row has been outdated *by the current transaction*, and throw error if
not.

And, if you accept the statement of the fix like that, then actually
there is no performance hit because there is no need to introduce new
tests. All we have to do is start treating HeapTupleSelfUpdated result
from heap_update or heap_delete as an error case instead of an
okay-do-nothing case. There doesn't even need to be an explicit check
that this was caused by a trigger, because AFAICS there isn't any other
possibility.

regards, tom lane