Re: [HACKERS] MERGE SQL Statement for PG11

On Fri, Apr 6, 2018 at 1:30 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:

>
>
> My impression is that this simply shouldn't be going through
> nodeModifyTuple, but be it's own nodeMerge node. The trigger handling
> would need to be abstraced into execTrigger.c or such to avoid
> duplication. We're now going from nodeModifyTable.c:ExecModifyTable()
> into execMerge.c:ExecMerge(), back to
> nodeModifyTable.c:ExecUpdate/Insert(). To avoid ExecInsert() doing
> things that aren't appropriate for merge, we then pass an actionState,
> which neuters part of ExecUpdate/Insert(). This is just bad.
>
>
But wouldn't this lead to lot of code duplication? For example,
ExecInsert/ExecUpdate does a bunch of supporting work (firing triggers,
inserting into indexes just to name a few) that MERGE's INSERT/UPDATE needs
as well. Now we can possibly move these support routines to a new file, say
execModify.c and then let both Merge as well as ModifyTable node make use
of that. But the fact is that ExecInsert/ExecUpdate knows a lot about
ModifyTable already. So to separate ExecInsert/ExecUpdate from ModifyTable
will require significant refactoring AFAICS. I am not saying we can't do
that, but will have it's own consequences.

If we would not have refactored to move ExecMerge and friends to a new
file, then it may not have looked so odd (as you describe above). But I
think moving the code to a new file was a net improvement. May be we can
move ExecInsert/Update etc to a new file as I suggested, but still use the
ModifyTable to run Merge. There are many things common between them.
ModifyTable executes all DMLs and MERGE is just another DML which can run
all three.

Thanks,
Pavan

--
Pavan Deolasee http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services