RE: Parallel INSERT (INTO ... SELECT ...)

Поиск
Список
Период
Сортировка
От Hou, Zhijie
Тема RE: Parallel INSERT (INTO ... SELECT ...)
Дата
Msg-id 70e4af0353274067af08344a42723201@G08CNEXMBPEKD05.g08.fujitsu.local
обсуждение исходный текст
Ответ на RE: Parallel INSERT (INTO ... SELECT ...)  ("Hou, Zhijie" <houzj.fnst@cn.fujitsu.com>)
Ответы Re: Parallel INSERT (INTO ... SELECT ...)  (Greg Nancarrow <gregn4422@gmail.com>)
Re: Parallel INSERT (INTO ... SELECT ...)  (Greg Nancarrow <gregn4422@gmail.com>)
Список pgsql-hackers
Hi

I took a look at v12-0001 patch, here are some comments:

1.
+    /*
+     * Setup the context used in finding the max parallel-mode hazard.
+     */
+    Assert(initial_max_parallel_hazard == 0 ||
+           initial_max_parallel_hazard == PROPARALLEL_SAFE ||
+           initial_max_parallel_hazard == PROPARALLEL_RESTRICTED);
+    context.max_hazard = initial_max_parallel_hazard == 0 ?
+        PROPARALLEL_SAFE : initial_max_parallel_hazard;

I am not quiet sure when will " max_parallel_hazard == 0"
Does it means the case max_parallel_hazard_context not initialized ?


2.
Some tiny code style suggestions

+        if (con->contype == CONSTRAINT_CHECK)
+        {
+            char       *conbin;
+            Datum        val;
+            bool        isnull;
+            Expr       *check_expr;

How about :

if (con->contype != CONSTRAINT_CHECK)
    continue;

3.
+                if (keycol == 0)
+                {
+                    /* Found an index expression */
+
+                    Node       *index_expr;

Like 2, how about:

If (keycol != 0)
    Continue;


4.
+            ListCell   *index_expr_item = list_head(index_info->ii_Expressions);
...
+                    index_expr = (Node *) lfirst(index_expr_item);
+                    index_expr = (Node *) expression_planner((Expr *) index_expr);

It seems BuildIndexInfo has already called eval_const_expressions for ii_Expressions,
Like the flow: BuildIndexInfo--> RelationGetIndexExpressions--> eval_const_expressions

So, IMO, we do not need to call expression_planner for the expr again.


And there seems another solution for this:

In the patch, We only use the { ii_Expressions , ii_NumIndexAttrs , ii_IndexAttrNumbers } from the IndexInfo,
which seems can get from "Relation-> rd_index".

Based on above, May be we do not need to call BuildIndexInfo to build the IndexInfo.
It can avoid some unnecessary cost.
And in this solution we do not need to remove expression_planner.

What do you think ?

Best regards,
houzj















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

Предыдущее
От: Michael Paquier
Дата:
Сообщение: Re: OpenSSL connection setup debug callback issue
Следующее
От: Justin Pryzby
Дата:
Сообщение: Re: PoC/WIP: Extended statistics on expressions