On Mon, Apr 2, 2018 at 10:40 PM, Peter Geoghegan <pg@bowt.ie> wrote: > On Mon, Apr 2, 2018 at 7:18 PM, Andres Freund <andres@anarazel.de> wrote: >> I did a scan through this, as I hadn't been able to keep with the thread >> previously. Sorry if some of the things mentioned here have been >> discussed previously. I am just reading through the patch in its own >> order, so please excuse if there's things I remark on that only later >> fully make sense. >> >> >> later update: TL;DR: I don't think the parser / executor implementation >> of MERGE is architecturally sound. I think creating hidden joins during >> parse-analysis to implement MERGE is a seriously bad idea and it needs >> to be replaced by a different executor structure. > > +1. I continue to have significant misgivings about this. It has many > consequences that we know about, and likely quite a few more that we > don't.
+1. I didn't understand from Peter's earlier comments that we were doing that, and I agree that it isn't a good design choice.
Honestly I don't think Peter ever raised concerns about the join, though I could be missing early discussions when I wasn't paying attention. It's there from day 1. Peter raised concerns about the two RTE stuff which was necessitated when we added support for partitioned table. We discussed that at some length, with your inputs and agreed that it's not necessarily a bad thing and probably the only way to deal with partitioned tables.
Personally, I don't see why an internal join is bad. That's what MERGE is doing anyways, so it closely matches with the overall procedure.