Re: support for MERGE

Поиск
Список
Период
Сортировка
От Zhihong Yu
Тема Re: support for MERGE
Дата
Msg-id CALNJ-vQQP0B28N57Hv1GOKDfvrcfN38FykV07EFq+RqzA4_ueQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: support for MERGE  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Ответы Re: support for MERGE  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Список pgsql-hackers


On Fri, Nov 12, 2021 at 3:13 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
On 2021-Nov-12, Zhihong Yu wrote:

> Hi,
>
> +           skipped_path = total - insert_path - update_path - delete_path;
>
> Should there be an assertion that skipped_path is not negative ?

Hm, yeah, added.

> +    * We maintain separate transaction tables for UPDATE/INSERT/DELETE since
> +    * MERGE can run all three actions in a single statement. Note that UPDATE
> +    * needs both old and new transition tables
>
> Should the 'transaction' in the first line be transition ?

Oh, of course.

Uploaded fixup commits to
https://github.com/alvherre/postgres/commits/merge-15

--
Álvaro Herrera              Valdivia, Chile  —  https://www.EnterpriseDB.com/
Hi,

+   resultRelInfo->ri_notMatchedMergeAction = NIL;

ri_notMatchedMergeAction -> ri_unmatchedMergeAction 

+static void ExecMergeNotMatched(ModifyTableState *mtstate,

ExecMergeNotMatched -> ExecMergeUnmatched

+    *    In this case, we are still dealing with a WHEN MATCHED case.
+    *    In this case, we recheck the list of WHEN MATCHED actions from

It seems the comment can be simplified to:

+    *    In this case, since we are still dealing with a WHEN MATCHED case,
+    *    we recheck the list of WHEN MATCHED actions from

+                                * If we got no tuple, or the tuple we get has

'get' appears in different tenses. Better use either 'get' or 'got' in both places.

+lmerge_matched:
...
+   foreach(l, resultRelInfo->ri_matchedMergeAction)

I suggest expanding the foreach macro into the form of for loop where the loop condition has extra boolean variable merge_matched.
Initial value for merge_matched can be true.
Inside the loop, we can adjust merge_matched's value to control whether the for loop continues.
This would avoid using goto label.

+       if (commandType == CMD_UPDATE && tuple_updated)

Since commandType can only be update or delete, it seems tuple_updated and tuple_deleted can be consolidated into one boolean variable (tuple_modified).
The above point is personal preference.

+        * We've activated one of the WHEN clauses, so we don't search
+        * further. This is required behaviour, not an optimization.
+        */
+       break;

We can directly return instead of break'ing.

+    * Similar logic appears in ExecInitPartitionInfo(), so if changing
+    * anything here, do so there too.

The above implies code dedup via refactoring - can be done in a separate patch.

To be continued ...

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Vik Fearing
Дата:
Сообщение: Re: BUFFERS enabled by default in EXPLAIN (ANALYZE)
Следующее
От: Zhihong Yu
Дата:
Сообщение: Re: support for MERGE