Re: support for MERGE

On Wed, Mar 9, 2022 at 9:38 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
wrote:

> I attach MERGE v14. This includes a fix from Amit Langote for the
> problem I described previously, with EvalPlanQual not working correctly.
> (I had failed to short-circuit the cross-partition update correctly.)
> Now the test case is enabled again, and it passes with the correct
> output.
>
> 0001 is as before; the only change is that I've added a commit message.
> Since this just moves some code around, I intend to get it pushed very
> soon.
>
> 0002 is the ExecUpdate/ExecDelete split. I cleaned up the struct
> definitions a bit more, but it's pretty much as in the previous version.
>
> 0003 is the actual MERGE implementation. Many previous review comments
> have been handled, and several other things are cleaner than before.
> I haven't made any changes for work_mem handling in ModifyTable's
> transition tables, though, and haven't attempted to address concurrent
> INSERT.
>
> --
> Álvaro Herrera 39°49'30"S 73°17'W —
> https://www.EnterpriseDB.com/

Hi,
For v14-0002-Split-ExecUpdate-and-ExecDelete-in-reusable-piec.patch :

+ TupleTableSlot *insertProjectedTuple;

With `insert` at the beginning of the variable name, someone may think it
is a verb. How about naming the variable projectedTupleFromInsert (or
something similar) ?

+ if (!ExecBRDeleteTriggers(context->estate, context->epqstate,
+ resultRelInfo, tupleid, oldtuple,
+ epqreturnslot))
+ /* some trigger made the delete a no-op; let caller know */
+ return false;

It seems you can directly return the value returned
from ExecBRDeleteTriggers().

+ if (!ExecBRUpdateTriggers(context->estate, context->epqstate,
+ resultRelInfo, tupleid, oldtuple, slot))
+ /* some trigger made the update a no-op; let caller know */
+ return false;

Similar comment as above (the comment is good and should be kept).

Cheers