Re: Breakage in trigger.c

Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> On Mon, Sep 06, 2004 at 05:55:29PM -0400, Tom Lane wrote:
>> Finally, surely the "Mark the event done" case should advance
>> prev_event? As-is the code is capable of messing up the list links.

> Not sure. Why doesn't the other arm of that conditional need it?

In that arm the event is being deleted from the list. prev_event should
always point to the last nondeleted event.

(BTW, seems this code is missing a bet. If the same subtransaction both
adds and fires the event, then couldn't we delete it immediately? That
would extend the immediately-deletable case to many more situations.)

> All in all I think this highlights a big shortcoming of this project.
> If there was much more extensive regression testing, this code could be
> refactored with much more confidence, and this kind of bugs wouldn't be
> there on the first place.

Regression testing is unlikely to expose small performance bugs, which
is what the first one is. The third one is only really going to come to
light if you test a case where a single query inside a subtransaction
adds both deletable and nondeletable entries (and does so in the right
order). Again I'm unconvinced that regression tests could be expected
to catch this; we may have tests that exercise such cases, but they aren't
doing it inside a subtransaction --- and only someone who was already
aware of the potential for a bug in this spot would be likely to think
to try it.

In short I have little confidence in "more tests!" as a panacea for
anything.

regards, tom lane