Re: support for MERGE

On Sat, 12 Mar 2022 at 11:53, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> On 2022-Mar-09, Zhihong Yu wrote:
>
>> 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) ?
>
> I went with projectedFromInsert. I don't feel a need to call it a
> "tuple" because it's already a TupleTableSlot * ...
>
>> + 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().
>
> True, let's do that.
>
> On 2022-Mar-10, Simon Riggs wrote:
>
>> Various cases were not tested in the patch - additional patch
>> attached, but nothing surprising there.
>
> Thanks, incorporated here.
>
> This is mostly just a rebase to keep cfbot happy.

Hi, hackers

+ ar_delete_trig_tcs = mtstate->mt_transition_capture;
+ if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_capture &&
+ mtstate->mt_transition_capture->tcs_update_old_table)
+ {
+ ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple,
+ NULL, NULL, mtstate->mt_transition_capture);
+
+ /*
+ * We've already captured the NEW TABLE row, so make sure any AR
+ * DELETE trigger fired below doesn't capture it again.
+ */
+ ar_delete_trig_tcs = NULL;
+ }

Maybe we can use ar_delete_trig_tcs in if condition and ExecARUpdateTriggers() to make the code shorter.

+ /* "old" transition table for DELETE, if any */
+ Tuplestorestate *old_del_tuplestore;
+ /* "new" transition table INSERT, if any */
+ Tuplestorestate *new_ins_tuplestore;

Should we add "for" for transition INSERT comment?

+MERGE is a multiple-table, multiple-action command: it specifies a target
+table and a source relation, and can contain multiple WHEN MATCHED and
+WHEN NOT MATCHED clauses, each of which specifies one UPDATE, INSERT,
+UPDATE, or DO NOTHING actions. The target table is modified by MERGE,
+and the source relation supplies additional data for the actions

I think the "source relation" is inaccurate. How about using "data source" like document?

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.