Re: [HACKERS] MERGE SQL Statement for PG11

On 2 February 2018 at 01:59, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> On Thu, Feb 1, 2018 at 11:39 AM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>> There is also the matter of subselects in the update targetlist, which
>> you similarly claim is only a problem of having the wrong error
>> message. The idea that those are unsupported for any principled reason
>> doesn't have any justification (unlike WHEN ... AND quals, and their
>> restrictions, which I agree are necessary). It clearly works with
>> Oracle, for example:
>>
>> http://sqlfiddle.com/#!4/2d5405/10
>>
>> You're reusing set_clause_list in the grammar, so I don't see why it
>> shouldn't work within MERGE in just the same way as it works in
>> UPDATE.
>
> Actually, I now wonder if there is a good reason for restrictions
> (e.g. no subselects) on WHEN ... AND quals, too. See this SQL fiddle
> from SQL Server:
>
> http://sqlfiddle.com/#!18/8acef/27
>
> 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.

> Most
> surprisingly of all, even the main join ON condition itself can have
> subselects (though that's probably a bad idea).

That should be supported, though I can't think of why you'd want that either.

> 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.

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.

>>> + * As top-level statements INSERT, UPDATE and DELETE have a Query,
>>> + * whereas with MERGE the individual actions do not require
>>> + * separate planning, only different handling in the executor.
>>> + * See nodeModifyTable handling of commandType CMD_MERGE.
>>> + *
>>> + * A sub-query can include the Target, but otherwise the sub-query
>>> + * cannot reference the outermost Target table at all.
>>> + */
>>> + qry->querySource = QSRC_PARSER;
>>> + joinexpr = makeNode(JoinExpr);
>>> + joinexpr->isNatural = false;
>>> + joinexpr->alias = NULL;
>>> + joinexpr->usingClause = NIL;
>>> + joinexpr->quals = stmt->join_condition;
>
> 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.

> Note that I consider cleaning
> up the Query and plan representations to be prerequisite to commit.

You'll need to be more specific. Later patches do move some things.

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