Re: UPSERT wiki page, and SQL MERGE syntax

From: Peter Geoghegan <pg(at)heroku(dot)com>
To: Kevin Grittner <kgrittn(at)ymail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>
Subject: Re: UPSERT wiki page, and SQL MERGE syntax
Date: 2014-10-03 22:42:04
Message-ID: CAM3SWZRnFxAHKrxYXrCLYOvQsEqewaNkF2t_tYvYO_7Lnk27Ug@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 3, 2014 at 2:44 PM, Kevin Grittner <kgrittn(at)ymail(dot)com> wrote:
> I've never seen "atomic" used to mean "you never get an error"
> before.

> When you are saying "atomic" you mean something quite different.

Perhaps I should have been more careful on that point. Just to be
crystal clear: I don't intend that you'll never get an error (some
cases clearly *should* error). But I want confidence that reasonable,
simple UPSERTs will never error due to a duplicate violation that they
thought the statement was supposed to avoid. That's the point of
UPSERT. Guarantees matter. I would like to make guarantees that are at
least as good as the upsert looping subxact pattern.

Teradata refers to an "atomic UPSERT". It also has a MERGE command.

>> My complaint is quite straightforward, really. I don't want to
>> have to tell users to do this: http://stackoverflow.com/a/22777749
>
> I think you are confusing syntax with semantics. I grant that a
> reasonable person could have concerns about using the MERGE syntax
> to implement the semantics you want in the special case that an
> appropriate unique index exists, but pretending that the semantics
> can't be achieved with that syntax is just silly.

I am not pretending that it can't be done - just, as I said, that to
do so would have bad consequences when time came to generalize the
syntax to support more advanced cases.

>> At the same time, I also don't want to have to live with the
>> consequences of implementing a MERGE that does not exhibit that
>> behavior. Which is to say, the consequences of doing something
>> like selectively using different types of snapshots (MVCC or
>> dirty - the two different ideas of "each row" that are in
>> tension) based on the exact clauses used. That seems like asking
>> for trouble, TBH.
>
> Now *that* is getting more to a real issue.

Yes, it is. I am opposed to using the MERGE syntax for this *because*
MERGE is useful (independently useful, when done properly), not
because it is not useful.

> We routinely pick very
> different plans based on the presence or absence of an index, and
> we use special snapshots in the course of executing many DML
> statements (if FK triggers are fired), but this would be the first
> place I can think of that the primary DML statement behaves that
> way. You and a couple others have expressed concern about that,
> but it seems pretty vague and hand-wavey. If a different execution
> node is used for the special behavior, and that node is generated
> based on the unique index being used, I'm really having problems
> seeing where the problem lies.

We need to avoid duplicate violations. The authoritative source of
truth here is a unique index, because the presence or absence of a
conclusively-visible conflict tuple controls whether our insert fails
or succeeds respectively. We need a dirty snapshot to see tuples not
in our MVCC snapshot, since they need to be handled too. Otherwise,
when we fail to handle them, the MERGE's INSERT has a dup violation
(which is not what I want, for practical reasons).

If we're seq scanning a table, even with a dirty snapshot, there is no
authoritative source of truth. Heap tuples will be inserted
concurrently, and we'll have no idea what to do about it without
reference to a unique index as a choke point/single source of final
truth about what is and isn't going to be duplicated.

As implemented, my UPSERT patch will have UPSERTs not really use their
MVCC snapshot for simple cases, just like INSERTs in general. UPDATEs
always use their MVCC snapshot, as the ModifyTable node pulls up
tuples from an ordinary relation scan. UPSERT needs a DirtySnapshot
scan of the unique index to find duplicates, because that's always how
we find duplicates. At the same time, MERGE is not at liberty to not
work just because of the absence of a unique index (plus even row
locking must be prepared for conflicts). And since the MERGE is driven
by an outer join - *not* an INSERT-like search for duplicates - we'll
find ourselves providing one behavior when detecting one case, and
another when detecting the other, with subtle and confusing
differences between each case. If we "fake" an outer join by doing an
INSERT-like search for duplicates in a unique index (to make sure we
see duplicates), then we get into trouble elsewhere. Like, we cannot
let users not ask for an INSERT in their MERGE, because we'd be
conclusively deciding to *not* (say) UPDATE on the basis of the
absence of a value in a unique index, as opposed to the presence.

Suddenly, we're in the business of proving a negative....or are we?.
My head hurts.

--
Peter Geoghegan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2014-10-03 22:43:18 Re: UPSERT wiki page, and SQL MERGE syntax
Previous Message Andres Freund 2014-10-03 22:21:23 Re: Fixed xloginsert_locks for 9.4