Re: pgsql: New files for MERGE

Hi,

On 2018-04-06 12:22:09 -0400, Bruce Momjian wrote:
> On Fri, Apr  6, 2018 at 09:21:54AM +0100, Simon Riggs wrote:
> > On 5 April 2018 at 21:02, Bruce Momjian <bruce@momjian.us> wrote:
> > > Simon, you have three committers in this thread suggesting this patch be
> > > reverted.  Are you just going to barrel ahead with the fixes without
> > > addressing their emails?
> > 
> > PeterG confirms that the patch works and has the agreed concurrency
> > semantics. Separating out the code allows us to see clearly that we
> > have almost complete test coverage of the code and its features.
> 
> My point was that people didn't ask you to work harder on fixing the
> patch, but in reverting it.  You can work harder on fixing things in the
> hope they change their minds, but again, that isn't addressing their
> request.

I concur with this description.  And you (Simon) have not at al reacted
to those points, instead very selectively quoting other parts of
relevant emails and responding to those.



> The architecture of the executor has been described as wrong, but at
> the time that was made there was zero detail behind that claim, so
> there was nothing to comment upon. I'm surprised and somewhat
> suspicious that multiple people found anything with which to agree or
> disagree with that suggestion; I'm also suprised that nobody else has
> pointed out the lack of substance there.

I started looking at the patch because I became suspicious by the way
this was committed the first time round.  It was a brief skim of the
patch, so I could gauge the situation accurately. The fact that you
could commit any time of it was obviously a factor, so I obviously
didn't have time (nor responsibility!) to have a detailed description of
how it should like.  You then went ahead six hours later, committing it,
without even referring, not to mention responding, to my points.

There were several obviously things wrong in the patch. As the extreme
example, a non-executor node file was named as an executor node. That's
hardly something hidden in a corner, you've seperately committed that
change twice.  The parse structure issue was also fairly clear.  You
can't say that those aren't things one couldn't have found in careful
pre-commit review.  That, and the fact the commit was broken in a broken
(and non-compiling the second time round) way twice, does make this
appear rushed.


> And as Robert has reminded me often that committers do not possess a
> veto that they can simply use to remove patches, there have been no
> reasonable grounds to revoke anything.

Nor does a single committer overrule three other committers raising
doubts. Without even xresponding to their concerns.


> It's been said that neither Tom or Andres thought the code would be
> committed so neither looked at this code, even though they both knew
> of my proposed commit of the feature in January, with some caveats at
> that time.

The patch looked *quite* different in January, so even if that were
true, what are you proposing that people do? Constantly look at features
that you say you'll commit in a few months? The state back then was
definitely not committable, so I don't even know what to make of this.

Greetings,

Andres Freund