Horton

Re: MERGE ... RETURNING

Dean Rasheed <dean.a.rasheed@gmail.com>

View thread
From: Dean Rasheed <dean.a.rasheed@gmail.com>
To: jian he <jian.universality@gmail.com>
Cc: Jeff Davis <pgsql@j-davis.com>, Vik Fearing <vik@postgresfriends.org>, Gurjeet Singh <gurjeet@singh.im>, Isaac Morland <isaac.morland@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-01-29T11:38:10Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add RETURNING support to MERGE.

  2. doc: Improve a couple of places in the MERGE docs.

  3. doc: improve description of privileges for MERGE and update glossary.

  4. Fix RLS policy usage in MERGE.

  5. Fix leak of LLVM "fatal-on-oom" section counter.

  6. Implement outer-level aggregates to conform to the SQL spec, with

Attachments

On Sun, 28 Jan 2024 at 23:50, jian he <jian.universality@gmail.com> wrote:
>
> one minor white space issue:
>
> git diff --check
> doc/src/sgml/func.sgml:22482: trailing whitespace.
> + action | clause_number | product_id | in_stock | quantity
>

Ah, well spotted! I'm not in the habit of running git diff --check.

> @@ -3838,7 +3904,7 @@ ExecModifyTable(PlanState *pstate)
>   }
>   slot = ExecGetUpdateNewTuple(resultRelInfo, context.planSlot,
>   oldSlot);
> - context.relaction = NULL;
> + node->mt_merge_action = NULL;
>
> I wonder what's the purpose of setting node->mt_merge_action to null ?
> I add `node->mt_merge_action = NULL;` at the end of each branch in
> `switch (operation)`.
> All the tests still passed.

Good question. It was necessary to set it to NULL there, because code
under ExecUpdate() reads it, and context.relaction would otherwise be
uninitialised. Now though, mtstate->mt_merge_action is automatically
initialised to NULL when the ModifyTableState node is first built, and
only the MERGE code sets it to non-NULL, so it's no longer necessary
to set it to NULL for other types of operation, because it will never
become non-NULL unless mtstate->operation is CMD_MERGE. So we can
safely remove that line.

Having said that, it seems a bit ugly to be relying on mt_merge_action
in so many places anyway. The places that test if it's non-NULL should
more logically be testing whether mtstate->operation is CMD_MERGE.
Doing that, reduces the number of places in nodeModifyTable.c that
read mt_merge_action down to one, and that one place only reads it
after testing that mtstate->operation is CMD_MERGE, which seems neater
and safer.

Regards,
Dean