Re: support for MERGE

On Fri, Jan 28, 2022 at 2:19 PM Andres Freund <andres(at)anarazel(dot)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.gar3j26gsk32gqpe@alap3.anarazel.de
> [2]
> https://www.postgresql.org/message-id/20180403021800.b5nsgiclzanobiup@alap3.anarazel.de
>
>
>