Re: INSERT ... ON CONFLICT {UPDATE | IGNORE} - Mailing list pgsql-hackers

On 09/26/2014 12:13 AM, Peter Geoghegan wrote:
> On Thu, Sep 25, 2014 at 1:48 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> C. Internal weirdness
>> Submit C based upon earlier agreed B, submit to Dec 15 CF, major patch
>> deadline, so we can fine tune for last CF.
>> Then Heikki rewrites half your patch in a better way, you thank him
>> and then we commit. All done.
>
> I don't have a problem with Heikki or anyone else rewriting the value
> locking part of the patch, provided it meets my requirements for such
> a mechanism. Since Heikki already agreed that that standard should be
> imposed, he'd hardly take issue with it now.
>
> However, the fact is that once you actually make something like
> promise tuples meet that standard, at the very least it becomes a lot
> messier than you'd think. Heikki's final prototype "super deleted"
> tuples by setting their xmin to InvalidTransactionId. We weren't sure
> that that doesn't break some random other heapam code. Consider this,
> for example:
>
> https://github.com/postgres/postgres/blob/REL9_4_STABLE/src/backend/executor/execMain.c#L1961
>
> So that looks safe in the face of setting xmin to InvalidTransactionId
> in the way the later prototype patch did if you think about it for a
> while, but there are other places where that is less clear. In short,
> it becomes something that we have to worry about for ever, because
> "xmin cannot change without the tuple in the slot changing" is clearly
> an invariant for certain purposes. It might accidentally fail to fail
> right now, but I'm not comfortable with it.

Just to be clear: I wrote the initial patch to demonstrate what I had in 
mind, because I was not able to explain it well enough otherwise. You 
pointed out issues with it, which I then fixed. You then pointed out 
more issues, which I then fixed again.

My patch version was a proof of concept, to demonstrate that it can be 
done. What I'd like you to do now, as the patch author, is to take the 
promise tuple approach and clean it up. If the xmin stuff is ugly, 
figure out some other way to do it.

> Now, I might be convinced that that's actually the way to go. I have
> an open mind. But that will take discussion. I like that page
> hwlocking is something that many systems do (even including Oracle, I
> believe). Making big changes to nbtree is always something that
> deserves to be met with skepticism, but it is nice to have an
> implementation that lives in the head of AM.

I don't know what you mean by "in the head of AM", but IMO it would be 
far better if we can implement this outside the index AMs. Then it will 
work with any index AM.

BTW, in the discussions, you pointed out that exclusion constraints 
currently behave differently from a unique index, when two backends 
insert a tuple at the same time. With a unique index, one of them will 
fail, but one is always guaranteed to succeed. With an exclusion 
constraint, they can both fail if you're unlucky. I think the promise 
tuples would allow us to fix that, too, while we're at it. In fact, you 
might consider tackling that first, and build the new INSERT ON CONFLICT 
syntax on top of that. Basically, an INSERT to a table with an exclusion 
constraint would be the same as "INSERT ON CONFLICT throw an error". 
That would be a useful way to split this patch into two parts.

- Heikki