Re: remove upsert example from docs
Bruce Momjian <bruce@momjian.us>
From: Bruce Momjian <bruce@momjian.us>
To: Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi>
Cc: Merlin Moncure <mmoncure@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2011-02-17T18:37:01Z
Lists: pgsql-hackers
Attachments
- /rtmp/upsert.diff (text/x-diff) patch
Marko Tiikkaja wrote: > 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. Good analysis. Documentation patch attached and applied. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +