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

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. +