On Thu, Mar 22, 2018 at 1:06 AM, Simon Riggs <si...@2ndquadrant.com> wrote: > I'm now proposing that I move to commit this, following my own final > review, on Tues 27 Mar in 5 days time, giving time for cleanup of > related issues. > > If there are any items you believe are still open, please say so now > or mention any other objections you have.
I would like to see clarity on the use of multiple RTEs for the target table by MERGE. See my remarks to Pavan just now. Also, I think that the related issue of how partitioning was implemented needs to get a lot more review (partitioning is the whole reason for using multiple RTEs, last I checked). It would be easy enough to fix the multiple RTEs issue if partitioning wasn't a factor. I didn't manage to do much review of partitioning at all. I don't really understand how the patch implements partitioning. Input from a subject matter expert might help matters quite a lot. Pavan hasn't added support for referencing CTEs, which other database systems with MERGE have. I think that it ought to be quite doable. It didn't take me long to get it working myself, but there wasn't follow through on that (I could have posted the patch, which looked exactly as you'd expect it to look). I think that we should add support for CTEs now, as I see no reason for the omission. In general, I still think that this patch could do with more review, but I'm running out of time. If you want to commit it, I will not explicitly try to block it, but I do have misgivings about your timeframe. -- Peter Geoghegan