On 4 April 2018 at 20:09, Tom Lane <t...@sss.pgh.pa.us> wrote: > [ removing -committers from cc ] > > Pavan Deolasee <pavan.deola...@gmail.com> writes: >> On Thu, Apr 5, 2018 at 12:16 AM, Andres Freund <and...@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
