Re: [HACKERS] MERGE SQL Statement for PG11

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Pavan Deolasee <pavan(dot)deolasee(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] MERGE SQL Statement for PG11
Date: 2018-01-30 09:45:49
Message-ID: CANP8+j+rO5jnDATve3gokRf6Jhfss5DquFxgz74rhJero8sAGQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 29 January 2018 at 22:31, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:

> I don't think that there should be any error, so I can't say.

>> I argued that was possible and desirable, but you argued it was not
>> and got everybody else to agree with you. I'm surprised to see you
>> change your mind on that.
>
> You're mistaken. Nothing I've said here is inconsistent with my
> previous remarks on how we deal with concurrency.

Please see here

https://www.postgresql.org/message-id/20171102191636.GA27644%40marmot

On 2 November 2017 at 19:16, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>
>> So if I understand you correctly, in your view MERGE should just fail
>> with an ERROR if it runs concurrently with other DML?
>
>
> That's certainly my opinion on the matter. It seems like that might be
> the consensus, too.

You've changed your position, which is good, thanks. No problem at all.

The proposal you make here had already been discussed in detail by
Pavan and myself. My understanding of that discussion was that he
thinks it might be possible, but I had said we must stick to the
earlier agreement on how to proceed. I am willing to try to produce
fewer concurrent errors, since that was an important point from
earlier work.

My only issue now is to make sure this does not cause confusion and
ask if that changes the views of others.

> According to your documentation, "MERGE provides a single SQL
> statement that can conditionally INSERT, UPDATE or DELETE rows, a task
> that would otherwise require multiple procedural language statements".
> But you're introducing a behavior/error that would not occur in
> equivalent procedural client code consisting of an UPDATE followed by
> a (conditionally executed) INSERT statement when run in READ COMMITTED
> mode. You actually get exactly the concurrency issue that you cite as
> unacceptable in justifying your serialization error with such
> procedural code (when the UPDATE didn't affect any rows, only
> following EPQ walking the UPDATE chain from the snapshot-visible
> tuple, making the client code decide to do an INSERT on the basis of
> information "from the future").

"You're introducing a behavior/error"... No, I advocate nothing in the
patch, I am merely following the agreement made here:

https://www.postgresql.org/message-id/CA%2BTgmoYOyX4nyu9mbMdYTLzT9X-1RptxaTKSQfbSdpVGXgeAJQ%40mail.gmail.com

Robert, Stephen, may we attempt to implement option 4 as Peter now suggests?

> 4. Implement MERGE, while attempting to avoid concurrent ERRORs in
> cases where that is possible.

I will discuss in more detail at the Brussels Dev meeting and see if
we can achieve consensus on how to proceed.

v14 posted with changes requested by multiple people. Patch status:
Needs Review.

Current summary: 0 wrong answers; 2 ERRORs raised need better
handling; some open requests for change/enhancement.

I will open a wiki page to track open items by the end of the week.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-01-30 09:49:49 Re: FOR EACH ROW triggers on partitioned tables
Previous Message Amit Langote 2018-01-30 09:39:26 Re: non-bulk inserts and tuple routing