On Fri, Jan 28, 2022 at 2:19 PM Andres Freund <and...@anarazel.de> wrote:
> Hi, > > On 2022-01-28 17:27:37 -0300, Alvaro Herrera wrote: > > MERGE, v10. I am much more comfortable with this version; I have > > removed a bunch of temporary hacks and cleaned up the interactions with > > table AM and executor, which is something that had been bothering me for > > a while. The complete set of changes can be seen in github, > > https://github.com/alvherre/postgres/commits/merge-15 > > Any chance you could split this into something more reviewable? The overall > diff stats are: 102 files changed, 8589 insertions(+), 234 deletions(-) > thats > pretty hard to really review. Incremental commits don't realy help with > that. > > > > I am not aware of anything of significance in terms of remaining work > > for this project. > > One thing from skimming: There's not enough documentation about the > approach > imo - it's a complicated enough feature to deserve more than the one > paragraph > in src/backend/executor/README. > > > I'm still [1],[2] uncomfortable with execMerge.c kinda-but-not-really > being an > executor node. > > > > The one thing I'm a bit bothered about is the fact that we expose a lot > of > > executor functions previously static. I am now wondering if it would be > > better to move the MERGE executor support functions into > nodeModifyTable.c, > > which I think would mean we would not have to expose those function > > prototypes. > > I'm worried about the complexity of nodeModifyTuple.c as is. Moving more > code > Hi, I think you meant nodeModifyTable.c. And I agree with your comment (on not making nodeModifyTable.c bigger). Cheers > in there does not seem like the right call to me - we should do the > opposite > if anything. > > > A few inline comments below. No real review, just stuff noticed while > skimming > to see where this is at. > > > Greetings, > > Andres > > > [1] > https://www.postgresql.org/message-id/20180405200003.gar3j26gsk32g...@alap3.anarazel.de > [2] > https://www.postgresql.org/message-id/20180403021800.b5nsgiclzanob...@alap3.anarazel.de > > >
