Re: pgsql: New files for MERGE - Mailing list pgsql-hackers

On 4 April 2018 at 20:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> [ removing -committers from cc ]
>
> Pavan Deolasee <pavan.deolasee@gmail.com> writes:
>> On Thu, Apr 5, 2018 at 12:16 AM, Andres Freund <andres@anarazel.de> wrote:
>>> Hows that an explanation for just going ahead and committing? Without
>>> even commenting on why one thinks the pointed out issues are something
>>> that can be resolved later or somesuch?  This has an incredibly rushed
>>> feel to it.
>
>> Anyways, I think your reviews comments are useful and I've incorporated
>> most of those. Obviously certain things like creating a complete new
>> executor machinery is not practical given where we're in the release cycle
>> and I am not sure if that has any significant advantages over what we have
>> today.
>
> Well, what's on the table is reverting this patch and asking you to try
> again in the v12 cycle.  Given Andres' concerns about the executor design,
> and mine about the way the parsing end is built, there's certainly no way
> that that's all getting fixed by Saturday.  Given pretty much everybody's
> unhappiness with the way this patch was forced through at the last minute,
> I do not think you should expect that we'll say, "okay, we'll let you ship
> a bad version of MERGE because there's no more time in this cycle".

This version works, with agreed semantics, all fully tested and documented.

It's also neat and tight. Look how easy it was for Peter to add WITH
semantics on top of it.

And it's isolated, so its not a threat to anybody that doesn't choose
to use it. Users want it and will use this; if I didn't know that for
certain I wouldn't spend time on it.

> Personally, I didn't think we had consensus on whether the semantics
> are right, let alone on whether this is a satisfactory implementation
> code-wise.  I know I've never looked at the patch before today; I did not
> think it was close enough to being committed that I would need to.

I have rejected patches in the past for clearly stated reasons that
affect users. I regret that those people didn't discuss their designs
before they posted patches and serious holes were found. And in
response I provided clear design outlines of what was needed. That is
not what is happening here.

The normal way is to make review comments that allow change. Your
request for change of the parser data structures is fine and can be
done, possibly by Saturday

If saying "I'm unhappy with something" is sufficient grounds for
rejecting a patch, I'm surprised to hear it. There has been no
discussion of what exactly would be better, only that what we have is
somehow wrong, a point which both Pavan and I dispute, not least
because the executor has already been rewritten once at Peter's
request.

I was under no pressure at all to commit this. In my opinion this is a
good version of MERGE and that is why I committed it. If it were not,
I would have no hesitation in pushing back a year or more, if I knew
of technical reasons to do so.

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