References: <CANbhV-HgNB805QSikNTVSeFOV_jpfGokQpbboQjHntzo9auXRw@mail.gmail.com>
 <CALNJ-vTr=gR+jQNUAn68ozFf1eCpCc2599oo=bkEW_Rr5xo9WA@mail.gmail.com>
 <202203120353.sdof7mld5qfc@alvherre.pgsql>
User-agent: mu4e 1.4.14; emacs 27.1
From: Japin Li <japinli@hotmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Zhihong Yu <zyu@yugabyte.com>, Simon Riggs
 <simon.riggs@enterprisedb.com>, Pg Hackers
 <pgsql-hackers@lists.postgresql.org>, Tomas Vondra
 <tomas.vondra@enterprisedb.com>, Daniel Westermann <dwe@dbi-services.com>,
 Amit Langote <amitlangote09@gmail.com>, Justin Pryzby
 <pryzby@telsasoft.com>, Erik Rijkers <er@xs4all.nl>, Jaime Casanova
 <jcasanov@systemguards.com.ec>, Andres Freund <andres@anarazel.de>
Subject: Re: support for MERGE
In-reply-to: <202203120353.sdof7mld5qfc@alvherre.pgsql>
Date: Mon, 14 Mar 2022 23:16:11 +0800
Message-ID:
 <MEYP282MB166983FFEC67F51A72432617B60F9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: bc8567b5-f9c1-4493-a65a-08da05cd97f0
X-MS-TrafficTypeDiagnostic: ME2P282MB0019:EE_
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info:
	klnOIgv4j3h6NNvmCrVaa1EWphLo5P4MWIu4EYD9kIbJF5KtWHMPPF82owzkNb9vINtgAhn3GfKQEF0o1u/txv0dGQFoX2hF2kx4Yhk9+hE5reR6ylUEgir+0Dee+CxsH8AjSrF8y9k5ZtlFwBKxC5X7NK94L8wjaUahfvx4No2beQyZMC1H0WqBj9YLpmR4J6IMbhGE6XOHtp53Aj01YXa/qI/HfMhesIQyz5xf9zr9Wp7sdnmB0TmpC5ke07wBiA1qxvehmT7XMRM5ZPAoy3izd7FHQYdHK/ku28n4SI1bX+i7lOrwMaKARhI3veZ8CkE0B/4hwMKtIYojRQZ3pMb2TxIGEbLncqgT/pND2rYYyib/fh4wolEE9HxFmUMBkgwEK6nR1fKgLFTxU2KktMW/KU+nVjx6TYZ/VkwM1kecggFmOUC3529DQR6erW7fmmkEwHMeXvc2+1rbvTaumdfPXiIvOMC/+/sR3SKz8dsuI+wAyHh/wzZ9YdZoxULQPs3NQ/MVlTpGKzsW9D5/4qLTRqyZuSp3ZzFSiIMN5Mawq9ebRdM6xCXVs3oSY5hN4/pFuj4JilQGeGcHBYwxAg==
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?l45E9ZZU3Y7LB52vd//JtDo4PrrwU6YsQmlNKCAThhWR9NMVRIzhifO7fPMx?=
 =?us-ascii?Q?D9SMBBamil1hLNJKc/djHxzRtkCMCjULp4IPMJ6Nkzfv9uG/MWJh+hQUcN/Q?=
 =?us-ascii?Q?1TkNCuM/6ZqZKiv4No2jLBHh3f8au32TznCqESJXoni3LMgNj6XQGmMi5Fry?=
 =?us-ascii?Q?nAOC8knIOrBU6OITjwmHvU2Xm8QBxXtn+h/SMf+s+8cH2xZYhEDwlPZw017R?=
 =?us-ascii?Q?LQdCsGwDltjVnTCiulAm6t9uj64Qd9pnLMyZ80Injft1zB5M3il4WMEL6QiT?=
 =?us-ascii?Q?3S004kiBmM2ugWdYZJ41OY2LIkTuX8MqUB4nQhHg4SXGMgpSIcg3vd5TUCtu?=
 =?us-ascii?Q?QeGcaDZASpuHI5LvyP1YChPFSytRkM/HMdMGgrE1ggSMoN0Fvv68GX0qB97J?=
 =?us-ascii?Q?fSH0DYf23G53pfT/v8G3MqKmBaldV+9tZDJ+TQcoWmU8l/Ftk/qhFCV4ECRz?=
 =?us-ascii?Q?jGog3C8TPuzVMhppT7xuoAAbDoTtSaGcolx9yRiMYDTmNOmY0lByKb6yZ/YY?=
 =?us-ascii?Q?f+/PyC22vIApiljWHnlGUBoq6GaRrH9yXNM5sfpd78XztuzcF9pavnG4hwvh?=
 =?us-ascii?Q?jIhgJaq2iFtj+HCoC/4+3iZKHFlHcDNRBeaHSa7/UPki3cnl1ItTeNtEGQui?=
 =?us-ascii?Q?DrDi+EsA88uGR6GrOH09OzwIGXzJpirv2aeVBtV48hXKNUoFAIsz2kHQaTYJ?=
 =?us-ascii?Q?DK1gTSnuBHhzQvgIQkcqx9xqN3/HNiSyifI+D75QhXLBQkd/5r9KxEOcYyhQ?=
 =?us-ascii?Q?/FljkYDZ14VCRqKCWuh6wbk/aKsdBAECAcDAovqwTlljVi+I9c8CXh4Ms9q7?=
 =?us-ascii?Q?Nz/4S7BS4PmP0ED7nQwnKV9pwcVpuLOT58/9bGsdfhJQL0Rm5DpnAYIpbmnE?=
 =?us-ascii?Q?lT+0dWmZayQPA/rnA0OUENgtPoTfWATG1gX+i+LXzQ36BcxTUWjs3Kx748/Z?=
 =?us-ascii?Q?G6G5r5gpUk+T6sBt3PmyNhLatnUTOMIk9WvJJ81TdBZk5TZmzBZ9ZJTf86D2?=
 =?us-ascii?Q?3vTiy7WS1sE8viNBZPu5+9ScY8ZpU9tZDpEtWHs2A6SZ8KCm8yBFQJDoiHWr?=
 =?us-ascii?Q?c/WHDmXoEGhlNw0iihGWFMAoHOurr+SoAZ+hICPD+a4TKh26jf4rKeA4WgoN?=
 =?us-ascii?Q?IhgAphrwh3h4e+KE7T0DDJuWYWnCLZVCQg=3D=3D?=
X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-746f3.templateTenant
X-MS-Exchange-CrossTenant-Network-Message-Id: bc8567b5-f9c1-4493-a65a-08da05cd97f0
X-MS-Exchange-CrossTenant-AuthSource: MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2022 15:16:20.5817
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg:
	00000000-0000-0000-0000-000000000000
X-MS-Exchange-Transport-CrossTenantHeadersStamped: ME2P282MB0019
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: <https://www.postgresql.org/message-id/MEYP282MB166983FFEC67F51A72432617B60F9%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
Precedence: bulk


On Sat, 12 Mar 2022 at 11:53, Alvaro Herrera <alvherre@alvh.no-ip.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.