Re: INSERT...ON DUPLICATE KEY LOCK FOR UPDATE - Mailing list pgsql-hackers

On Fri, Oct 11, 2013 at 5:43 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>>> * The visibility hacks that V4 is likely to have. The fact that
>>>> preserving the composable syntax may imply changes to
>>>> HeapTupleSatisfiesMVCC() so that rows locked but with no currently
>>>> visible version (under conventional rules) are visible to our snapshot
>>>> by virtue of having been locked all the same (this only matters at
>>>> read committed).
>>>
>>> I continue to think this is a bad idea.

>> Is it just because of performance concerns? /

> That's part of it; but I also think that HeapTupleSatisfiesMVCC() is a
> pretty fundamental bit of the system that I am loathe to tamper with.
> We can try to talk ourselves into believing that the definition change
> will only affect this case, but I'm wary that there will be
> unanticipated consequences, or simply that we'll find, after it's far
> too late to do anything about it, that we don't particularly care for
> the new semantics.  It's probably an overstatement to say that I'll
> oppose any whatsoever that touches the semantics of that function, but
> not by much.

A tuple that is exclusively locked by our transaction and not updated
or deleted being visible on that basis alone isn't *that* hard to
reason about. Granted, we need to be very careful here, but we're
talking about 3 lines of code.

>> Do you see the appeal of the composable syntax?
>
> To some extent.  It seems to me that what we're designing is a giant
> grotty hack, albeit a convenient one.  But if we're not really going
> to get MERGE, I'm not sure how much good it is to try to pretend we've
> got something general.

Well, to be fair perhaps all of the things that you consider grotty
hacks seem like inherent requirements to me, for any half-way
reasonable upsert implementation on any system, that has the essential
property of upsert: an atomic insert-or-update (or maybe serialization
failure).

>> But that's simpler than any of the alternatives that I see.
>> Does there really need to be a new snapshot type with one tiny
>> difference that apparently doesn't actually affect conventional
>> clients of MVCC snapshots?
>
> I think that's the wrong way of thinking about it.  If you're
> introducing a new type of snapshot, or tinkering with the semantics of
> an existing one, I think that's a reason to reject the patch straight
> off.  We should be looking for a design that doesn't require that.  If
> we can't find one, I'm not sure we should do this at all.

I'm confused by this. We need to lock a row not visible to our
snapshot under conventional rules. I think we can rule out
serialization failures at read committed. That just leaves changing
something about the visibility rules of an existing snapshot type, or
creating a new snapshot type, no?

It would also be unacceptable to update a tuple, and not have the new
row version (which of course will still have "information from the
future") visible to our snapshot - what would regular RETURNING
return? So what do you have in mind? I don't think that locking a row
and updating it are really that distinct anyway. The benefit of
locking is that we don't have to update. We can delete, for example.

Perhaps I've totally missed your point here, but to me it sounds like
you're saying that certain properties must always be preserved that
are fundamentally in tension with upsert working in the way people
expect, and the way it is bound to actually work in numerous other
systems.

-- 
Peter Geoghegan