Re: INSERT ... ON CONFLICT {UPDATE | IGNORE}

On 29 September 2014 18:59, Peter Geoghegan <pg@heroku.com> wrote:
> On Mon, Sep 29, 2014 at 7:21 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> If you were an ORM developer reading the PostgreSQL Release Notes for
>> 9.5, which URL would you visit to see a complete description of the
>> new feature, including how it works concurrently, locking and other
>> aspects. How would you check whether some strange behaviour was a bug,
>> or intentional?
>
> We don't do that with UPDATE, so why would we do it with this?

Because this is new, harder and non-standard, so there is no other
place to look. If you want to persuade us that MERGE has poorly
defined concurrency, so you have implemented a new command, the new
command had better have very well defined behaviour.

And because a reviewer asked for it?

For example, this patch for UPSERT doesn't support updatable views.
But I can't see anyone that didn't read the patch would know that.


>>> All of these were added. There are two new sets of isolation tests,
>>> one per variant of the new clause (IGNORE/UPDATE).
>>
>> When you say "added", what do you mean? You posted one new doc patch,
>> with no tests in it.
>
> I mean that there was a commit (not included with the documentation,
> but with the original patchset) with many tests. I don't know why
> you're suggesting that I don't have "concurrency tests". There are
> isolation tests in that commit. There are also many regression tests.

I see the tests in earlier patches; I was observing there are no new ones.

There are no tests for the use of CONFLICTING() syntax
No tests for interaction with triggers, with regard to before triggers
changing values prior to conflict detection.

My hope was that the complex behaviour of multiple unique indexes
might be explained there. Forgive me, I didn't see it.



>>>> No explanation of why the CONFLICTING() syntax differs from OLD./NEW.
>>>> syntax used in triggers
>>>
>>> Why should it be the same?
>>
>> Because it would be a principled approach to do that.
>
> That is just an assertion. The MERGE syntax doesn't use that either.

MERGE allows "AS row" which then allow you to refer to row.x for
column x of the input.

Other people have independently commented the same thing.


>> If we aren't going to use MERGE syntax, it would make sense to at
>> least use the same terminology.
>>
>> e.g.
>> INSERT ....
>> WHEN MATCHED
>> UPDATE
>>
>> The concept of "matched" is identical between MERGE and UPSERT and it
>> will be confusing to have two words for the same thing.
>
> I don't care if we change the spelling to "WHEN MATCHED
> UPDATE/IGNORE". That seems fine. But MERGE is talking about a join,
> not the presence of a would-be duplicate violation.

I don't understand that comment.

>> There seems to be a good reason not to use the MySQL syntax of ON
>> DUPLICATE KEY UPDATE, which doesn't allow you to specify UPDATE
>> operations other than a replace, so no deltas, e.g. SET a = a + x
>
> That isn't true, actually. It clearly does.

It does. Rather amusingly I misread the very unclear MySQL docs.


>> Having said that, it would be much nicer to have a mode that allows
>> you to just say the word "UPDATE" and have it copy the data into the
>> correct columns, like MySQL does. That is very intuitive, even if it
>> isn't very flexible.
>
> Multi-assignment updates (with or without CONFLICTING()) are supported, FWIW.

If I want the incoming row to overwrite the old row, it would be good
to have syntax to support that easily.

Why doesn't
INSERT INTO UNIQUE_TBL VALUES (1, 'a'), (2, 'b'), (2, 'b')  ON
CONFLICT UPDATE SET t = 'fails';
end up with this in the table?

1   a
2   fails

What happens with this?

BEGIN;
INSERT INTO UNIQUE_TBL VALUES (2, 'b')  ON CONFLICT UPDATE SET t = 'fails';
INSERT INTO UNIQUE_TBL VALUES (2, 'b')  ON CONFLICT UPDATE SET t = 'fails';
COMMIT;

-- Simon Riggs                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services