Re: pgsql: New files for MERGE

2018-04-13 Thread David Steele
On 4/11/18 5:28 PM, Simon Riggs wrote: > On 11 April 2018 at 21:00, Simon Riggs wrote: > >> "Commence primary ignition." > > Revert patch attached > > Mostly Pavan, minor cleanup by me I have moved this entry to the next CF in Waiting for Author state. Regards, --

Re: pgsql: New files for MERGE

2018-04-11 Thread Michael Paquier
On Wed, Apr 11, 2018 at 10:28:10PM +0100, Simon Riggs wrote: > Requesting review so I don't cause multiple revert commits. I have gone through the code tree with your patch applied for some time, and it seems to me that there are no more traces of merge or any of its infrastructure. Tests are

Re: pgsql: New files for MERGE

2018-04-11 Thread Simon Riggs
On 7 April 2018 at 18:45, Tom Lane wrote: > Simon Riggs writes: >> On 6 April 2018 at 17:22, Bruce Momjian wrote: >>> My point was that people didn't ask you to work harder on fixing the >>> patch, but in reverting it. You can work

Re: pgsql: New files for MERGE

2018-04-07 Thread Andres Freund
Hi, On 2018-04-07 13:33:53 -0400, Bruce Momjian wrote: > To summarize how I see this patch, we have this workflow at the top of > the TODO list (which I think Simon helped with or suggested): > > Desirability -> Design -> Implement -> Test -> Review -> Commit > > I think the MERGE patch

Re: pgsql: New files for MERGE

2018-04-07 Thread Andres Freund
Hi, On 2018-04-07 13:45:21 -0400, Tom Lane wrote: > Simon Riggs writes: > > On 6 April 2018 at 17:22, Bruce Momjian wrote: > >> 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

Re: pgsql: New files for MERGE

2018-04-07 Thread Tom Lane
Simon Riggs writes: > On 6 April 2018 at 17:22, Bruce Momjian wrote: >> 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

Re: pgsql: New files for MERGE

2018-04-07 Thread Bruce Momjian
On Sat, Apr 7, 2018 at 06:19:19PM +0100, Simon Riggs wrote: > Now I can see some people are annoyed, so I'm happy to apologize if > I've done things that weren't understood or caused annoyance. Time is > short at end of CF and tempers fray for us all. > > If Tom or Andres still feel that their

Re: pgsql: New files for MERGE

2018-04-07 Thread Simon Riggs
On 6 April 2018 at 17:22, 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 wrote: >> > Simon, you have three committers in this thread suggesting this patch be >> > reverted. Are

Re: pgsql: New files for MERGE

2018-04-06 Thread Robert Haas
On Fri, Apr 6, 2018 at 7:37 AM, Tom Kincaid wrote: > So given all this, I am not sure why people feel this patch was rushed > through or has a flawed design. The comments from Andres while I am > sure they have merit came before the commit but technically after the >

Re: pgsql: New files for MERGE

2018-04-06 Thread Alexander Korotkov
On Fri, Apr 6, 2018 at 8:43 PM, Andres Freund wrote: > On 2018-04-06 20:39:26 +0300, Alexander Korotkov wrote: > > On Fri, Apr 6, 2018 at 7:56 PM, Andres Freund > wrote: > > > > > On 2018-04-06 12:22:09 -0400, Bruce Momjian wrote: > > > > And as Robert

Re: pgsql: New files for MERGE

2018-04-06 Thread Peter Geoghegan
On Fri, Apr 6, 2018 at 1:21 AM, Simon Riggs 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

Re: pgsql: New files for MERGE

2018-04-06 Thread Andres Freund
On 2018-04-06 20:39:26 +0300, Alexander Korotkov wrote: > On Fri, Apr 6, 2018 at 7:56 PM, Andres Freund wrote: > > > On 2018-04-06 12:22:09 -0400, Bruce Momjian wrote: > > > And as Robert has reminded me often that committers do not possess a > > > veto that they can simply

Re: pgsql: New files for MERGE

2018-04-06 Thread Alexander Korotkov
On Fri, Apr 6, 2018 at 7:56 PM, Andres Freund wrote: > On 2018-04-06 12:22:09 -0400, Bruce Momjian wrote: > > 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

Re: pgsql: New files for MERGE

2018-04-06 Thread Andres Freund
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 wrote: > > > Simon, you have three committers in this thread suggesting this patch be > > > reverted. Are you just

Re: pgsql: New files for MERGE

2018-04-06 Thread Bruce Momjian
On Fri, Apr 6, 2018 at 09:21:54AM +0100, Simon Riggs wrote: > On 5 April 2018 at 21:02, Bruce Momjian 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

Re: pgsql: New files for MERGE

2018-04-06 Thread Pavan Deolasee
On Fri, Apr 6, 2018 at 1:51 PM, Simon Riggs wrote: > Given that the executor > manifestly works and has been re-engineered according to PeterG's > requests and that many performance concerns have already been > addressed prior to commit, Pavan and I were happy with it. My

Re: pgsql: New files for MERGE

2018-04-06 Thread Tom Kincaid
On Thu, Apr 5, 2018 at 10:54 PM, Michael Paquier wrote: > On Thu, Apr 05, 2018 at 04:02:20PM -0400, Bruce Momjian 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 >>

Re: pgsql: New files for MERGE

2018-04-06 Thread Simon Riggs
On 5 April 2018 at 21:02, Bruce Momjian wrote: > On Thu, Apr 5, 2018 at 11:15:20AM +0100, Simon Riggs wrote: >> On 4 April 2018 at 21:28, Simon Riggs wrote: >> > On 4 April 2018 at 21:14, Andres Freund wrote: >> > >> >>> The normal

Re: pgsql: New files for MERGE

2018-04-05 Thread Michael Paquier
On Thu, Apr 05, 2018 at 04:02:20PM -0400, Bruce Momjian 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? If my opinion counts, please count me in this bucket as well.

Re: pgsql: New files for MERGE

2018-04-05 Thread Bruce Momjian
On Thu, Apr 5, 2018 at 11:15:20AM +0100, Simon Riggs wrote: > On 4 April 2018 at 21:28, Simon Riggs wrote: > > On 4 April 2018 at 21:14, Andres Freund wrote: > > > >>> The normal way is to make review comments that allow change. Your > >>> request for

Re: pgsql: New files for MERGE

2018-04-05 Thread Andres Freund
On 2018-04-05 06:09:31 -0400, Robert Haas wrote: > On Wed, Apr 4, 2018 at 3:09 PM, Tom Lane wrote: > > 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

Re: pgsql: New files for MERGE

2018-04-05 Thread Simon Riggs
On 4 April 2018 at 21:28, Simon Riggs wrote: > On 4 April 2018 at 21:14, Andres Freund wrote: > >>> 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,

Re: pgsql: New files for MERGE

2018-04-05 Thread Robert Haas
On Wed, Apr 4, 2018 at 3:09 PM, Tom Lane wrote: > 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 >

Re: pgsql: New files for MERGE

2018-04-04 Thread Michael Paquier
On Wed, Apr 04, 2018 at 10:10:46AM -0700, Andres Freund wrote: > This needs at the very least a response to the issues pointed out in the > referenced email that you chose to ignore without any sort of comment. That's definitely not cool. -- Michael signature.asc Description: PGP signature

Re: pgsql: New files for MERGE

2018-04-04 Thread Simon Riggs
On 4 April 2018 at 21:14, Andres Freund wrote: >> 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 > > I did request changes, and you've so far ignored those

Re: pgsql: New files for MERGE

2018-04-04 Thread Peter Geoghegan
On Wed, Apr 4, 2018 at 1:07 PM, Simon Riggs wrote: > This version works, with agreed semantics, all fully tested and documented. I agree that it's more or less true that this works, and implements the agreed-upon semantics. I also agree that that's very important. That's

Re: pgsql: New files for MERGE

2018-04-04 Thread Andres Freund
Hi, On 2018-04-04 21:07:25 +0100, Simon Riggs wrote: > It's also neat and tight. Look how easy it was for Peter to add WITH > semantics on top of it. Err. Several parts of the code definitely do not look "neat and tight". As detailed in my email. Possibly that's necessary, but you've not argued

Re: pgsql: New files for MERGE

2018-04-04 Thread Simon Riggs
On 4 April 2018 at 20:09, Tom Lane wrote: > [ removing -committers from cc ] > > Pavan Deolasee writes: >> On Thu, Apr 5, 2018 at 12:16 AM, Andres Freund wrote: >>> Hows that an explanation for just going ahead and committing?

Re: pgsql: New files for MERGE

2018-04-04 Thread Peter Geoghegan
On Wed, Apr 4, 2018 at 12:09 PM, Tom Lane wrote: > 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

Re: pgsql: New files for MERGE

2018-04-04 Thread Tom Lane
[ removing -committers from cc ] Pavan Deolasee writes: > On Thu, Apr 5, 2018 at 12:16 AM, Andres Freund wrote: >> Hows that an explanation for just going ahead and committing? Without >> even commenting on why one thinks the pointed out issues are

Re: pgsql: New files for MERGE

2018-04-04 Thread Pavan Deolasee
On Thu, Apr 5, 2018 at 12:16 AM, Andres Freund wrote: > Hi, > > On 2018-04-05 00:02:06 +0530, Pavan Deolasee wrote: > > Apologies from my end. Simon checked with me regarding your referenced > > email. I was in the middle of responding to it (with a add-on patch to > take > >

Re: pgsql: New files for MERGE

2018-04-04 Thread Peter Geoghegan
On Wed, Apr 4, 2018 at 11:46 AM, Andres Freund 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 >

Re: pgsql: New files for MERGE

2018-04-04 Thread Andres Freund
Hi, On 2018-04-05 00:02:06 +0530, Pavan Deolasee wrote: > Apologies from my end. Simon checked with me regarding your referenced > email. I was in the middle of responding to it (with a add-on patch to take > care of your review comments), but got side tracked by some high priority > customer

Re: pgsql: New files for MERGE

2018-04-04 Thread Pavan Deolasee
On Wed, Apr 4, 2018 at 10:40 PM, Andres Freund wrote: > Hi, > > On 2018-04-03 08:32:45 -0700, Andres Freund wrote: > > Hi, > > > > On 2018-04-03 09:24:12 +, Simon Riggs wrote: > > > New files for MERGE > > > src/backend/executor/nodeMerge.c | 575

Re: pgsql: New files for MERGE

2018-04-04 Thread Andres Freund
Hi, On 2018-04-03 08:32:45 -0700, Andres Freund wrote: > Hi, > > On 2018-04-03 09:24:12 +, Simon Riggs wrote: > > New files for MERGE > > src/backend/executor/nodeMerge.c | 575 +++ > > src/backend/parser/parse_merge.c | 660 > >

Re: pgsql: New files for MERGE

2018-04-03 Thread Andres Freund
Hi, On 2018-04-03 09:24:12 +, Simon Riggs wrote: > New files for MERGE > src/backend/executor/nodeMerge.c | 575 +++ > src/backend/parser/parse_merge.c | 660 > src/include/executor/nodeMerge.h | 22 + >