On 8/5/2010 9:44 PM, Merlin Moncure wrote:
> On Thu, Aug 5, 2010 at 2:09 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
>> I was not persuaded that there's a real bug in practice. IMO, his
>> problem was a broken trigger not broken upsert logic. Even if we
>> conclude this is unsafe, simply removing the example is of no help to
>> anyone.
>
> Well, the error handler is assuming that the unique_volation is coming
> from the insert made within the loop. This is obviously not a safe
> assumption in an infinite loop context. It should be double checking
> where the error was being thrown from -- but the only way I can think
> of to do that is to check sqlerrm.
Yeah, this is a known problem with our exception system. If there was
an easy and reliable way of knowing where the exception came from, I'm
sure the example would include that.
> Or you arguing that if you're
> doing this, all dependent triggers must not throw unique violations up
> the exception chain?
If he isn't, I am. I'm pretty sure you can break every example in the
docs with a trigger (or a rule) you haven't thought through.
>> A more useful response would be to supply a correct example.
> Agree: I'd go further I would argue to supply both the 'safe' and
> 'high concurrency (with caveat)' way. I'm not saying the example is
> necessarily bad, just that it's maybe not a good thing to be pointing
> as a learning example without qualifications. Then you get a lesson
> both on upsert methods and defensive error handling (barring
> objection, I'll provide that).
The problem with the "safe" way is that it's not safe if called in a
transaction with isolation level set to SERIALIZABLE.
Regards,
Marko Tiikkaja