Re: WIP fix proposal for bug #6123

On Fri, Jul 22, 2011 at 5:01 PM, Kevin Grittner
<Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
>> Your scenario is a BEFORE DELETE trigger that does an UPDATE on
>> the same row, but I think this problem also occurs if you have a
>> BEFORE UPDATE trigger that does an UPDATE on the same row.  I
>> believe the second update gets silently ignored.
>
> My testing shows that the primary update gets ignored, while all the
> triggered effects of that update are persisted.  Yuck.  :-(

That was my recollection...

> It
> certainly seems possible to turn that around, but that hardly seems
> better.

Agreed.

> In asking application programmers here what they would
> *expect* to happen, they all seem to think that it is surprising
> that the BEFORE trigger functions *return a record*, rather than a
> boolean to say whether to proceed with the operation.  They feel it
> would be less confusing if a value set into NEW was effective if the
> operation does take effect, and the boolean controls whether or not
> that happens.  They rather expect that if an update happens from the
> same transaction while a before trigger is running, that the NEW
> record will reflect the change.

I think this is mostly a matter of what you get familiar with, and, as
you say, not worth breaking compatibility for.

> I recognize how hard it would be to create that expected behavior,
> and how unlikely it is that the community would accept such a change
> at this point.  But current behavior is to silently do something
> really dumb, so I think some change should be considered -- even if
> that change is to throw an error where we now allow nonsense.
>
> INSERT is not a problem -- if a BEFORE INSERT trigger inserts a row
> with a conflicting primary key (or other unique index key), the
> operation will be rolled back.  That's fine.
>
> I think DELETE can be cleanly fixed with a patch similar to what I
> posted earlier in the thread.  I found one more value that looks
> like it should be set, and it could use some comments, but I believe
> that we can get DELETE behavior which is every bit as sensible as
> INSERT behavior with a very small change.
>
> The worms do come crawling out of the can on BEFORE UPDATE triggers,
> though.  When faced with an UPDATE which hasn't yet been applied,
> and other UPDATEs triggering from within the BEFORE UPDATE trigger
> which touch the same row, it doesn't seem like you can honor both
> the original UPDATE which was requested *and* the triggered UPDATEs.
> Of course, if you discard the original UPDATE, you can't very well
> do anything with the record returned from the BEFORE UPDATE trigger
> for that update.  Since it seems equally evil to allow the update
> while ignoring some of the work caused by its trigger functions as
> to show the work of the triggered updates while suppressing the
> original update, I think the right thing is to throw an error if the
> old row for a BEFORE UPDATE is updated by the same transaction and
> the trigger function ultimately returns a non-NULL value.
>
> Thoughts?

Well, it seems to me that if the trigger update and the main update
were executed as separate commands (with no triggers involved) it
would often be the case that they'd dovetail nicely. When this has
come up for me, it's usually been the case that the sets of fields
being updated are completely non-overlapping. So ideally what I'd
like to happen is to have EPQ, or something like it, test whether the
newest version of the row still satisfies the UPDATE criteria. If so,
it applies the update to the new row version; if not, it either
discards the main UPDATE or throws an error. There's still some room
here for surprising results, but I think they would be surprising
results arising out of having done something intrinsically
complicated, rather than surprising results arising out of an odd
implementation artifact.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company