Re: [HACKERS] MERGE SQL Statement for PG11

On Sat, Feb 3, 2018 at 2:15 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> I started looking at SQL Server's MERGE to verify that it also does
>> not impose any restrictions on subselects in a MERGE UPDATE's
>> targetlist, just like Oracle. Unsurprisingly, it does not. More
>> surprisingly, I noticed that it also doesn't seem to impose
>> restrictions on what can appear in WHEN ... AND quals.
>
> You earlier agreed that subselects were not part of the Standard.

You know that I didn't say that, Simon.

>> What this boils down to is that I don't think that this part of your
>> design is committable (from your recent v14):
>
> So your opinion is that because v14 patch doesn't include a feature
> extension that is in Oracle and SQLServer that we cannot commit this
> patch.
>
> There are quite a few minor additional things in that category and the
> syntax of those two differ, so its clearly impossible to match both
> exactly.
>
> That seems like poor reasoning on why we should block the patch.

It certainly is. Good thing I never said anything of the sort.

There are 3 specific issues on query structure, that together paint a
picture about what you're not doing in the optimizer:

1. Whether or not subselects in the UPDATE targetlist are supported.

2. Whether or not subselects in the WHEN ... AND quals support subselects.

3. Whether or not subselects are possible within the main ON () join.

I gave a lukewarm endorsement of not supporting #3, was unsure with
#2, and was very clear on #1 as soon as I saw the restriction: UPDATE
targetlist in a MERGE are *not* special, and so must support
subselects, just like ON CONFLICT DO UPDATE, for example.

> If you would like to say how you think the design should look, it
> might be possible to change it for this release. Changing it in the
> future would not be blocked by commiting without that.

I can tell you right now that you need to support subselects in the
targetlist. They are *not* an extension to the standard, AFAICT. And
even if they were, every other system supports them, and there is
absolutely no logical reason to not support them other than the fact
that doing so requires significant changes to the data structures in
the parser, planner, and executor. Reworking that will probably turn
out to be necessary for other reasons that I haven't thought of.

I think that restrictions like this are largely an accident of how
your patch evolved. It would be a lot easier to work with you if you
acknowledged that.

>> I am willing to continue to engage with you on the concurrency issues
>> for the time being, since that is the most pressing issue for the
>> patch. We can get to this stuff later.
>
> There are no concurrency issues. The patch gives the correct answer in
> all cases, or an error to avoid problems. We've agreed that it is
> desirable we remove some of those if possible, though they are there
> as a result of our earlier discussions.

You seem to presume to be in charge of the parameters of this
discussion. I don't see it that way. I think that READ COMMITTED
conflict handling semantics are by far the biggest issue for the
patch, and that we should prioritize reaching agreement there. This
needs to be worked out through community consensus, since it concerns
fundamental semantics much more than implementation details. (In
contrast, the optimizer issues I mentioned are fairly heavy on
relatively uncontentious implementation questions.)

The problem with how you've represented MERGE in the parser,
optimizer, and executor is not that it's "half-baked crap", as you
suggested others might think at the FOSDEM developer meeting [1]. I
wouldn't say that at all. What I'd say is that it's *unfinished*. It's
definitely sufficient to prototype different approaches to
concurrency, as well as to determine how triggers should work, and
many other such things. That's a good start.

I am willing to mostly put aside the other issues for the time being,
to get the difficult questions on concurrency out of the way first.
But if you don't make some broad concessions on the secondary issues
pretty quickly, then I will have to conclude that our positions are
irreconcilable. I will have nothing further to contribute to the
discussion.

[1] https://wiki.postgresql.org/wiki/FOSDEM/PGDay_2018_Developer_Meeting#Minutes
--
Peter Geoghegan