Thread

Re: WIP fix proposal for bug #6123

Kevin Grittner <kevin.grittner@wicourts.gov> — 2011-08-01T18:02:49Z

"Kevin Grittner" <Kevin.Grittner@wicourts.gov> wrote:
 
> By the way, my current patch does break two existing UPDATE
> statements in the regression test misc.sql file:
 
Fixed in the attached version of the patch.
 
I consider the trigger behavior addressed by this patch to be a bug,
and serious enough to merit inclusion of a fix in the 9.1 release,
even at this late stage.  I don't think it should be back-patched,
because it changes behavior that there is some remote chance that
somebody somewhere understands and is intentionally using.  While I
agree that Robert's suggested approach (or something functionally
equivalent) would be the ideal long-term solution, I think that it
would be too large of a change to consider for 9.1.  I am suggesting
a minimal "defensive" change for 9.1, with possible development of a
fancier approach in a later release.
 
To recap: a trigger for UPDATE BEFORE EACH ROW or DELETE BEFORE EACH
ROW, which directly or indirectly causes update of the OLD row in
the trigger, can cause the triggering operation to be silently
ignored even though the trigger returns a non-NULL record, and even
though all triggered modifications are persisted.  The only clue
that an incomplete and incongruous set of operations has been
performed is that the UPDATE or DELETE count from the statement is
reduced by the number of rows on which this occurred: if an UPDATE
would have affected 5 rows, but one of them just fired triggers *as
though* it had been updated but was actually left untouched by the
original UPDATE statement, you would get "UPDATE 4" rather than
"UPDATE 5".
 
This patch causes DELETE to behave as most people would expect, and
throws and ERROR on the conflicting UPDATE case.
 
So far, beyond my confirmation that it passes all PostgreSQL
regression tests and works as expected in a few ad hoc tests I've
done, there have been six full-time days of business analysts here
testing our applications with a version of 9.0.4 patched this way,
confirming that the application works as expected.  They have a
standard set of testing scripts they use for every release, and
programmers have added tests specifically targeted at this area. 
Plus the analysts are trying to exercise as many paths to data
modification as possible, to stress the triggers, including
interfaces with other agencies.  They are scheduled to do a minimum
of 20 more full-time days of testing in the next two weeks, so if
someone wants to suggest changes or alternatives to this particular
patch, there would still be time to get over 100 hours of
professional testing against it -- if it comes through soon enough.
 
-Kevin