Re: Parallel INSERT (INTO ... SELECT ...) - Mailing list pgsql-hackers

On Fri, Jan 22, 2021 at 6:21 PM Hou, Zhijie <houzj.fnst@cn.fujitsu.com> wrote:
> Hi greg,
>
> Thanks for debugging this.
>
> May be I missed something. I am not sure about the case when rel->rd_index was NULL.
> Because, In function BuildIndexInfo, it seems does not have NULL-check for index->rd_index.
> Like the following:
> ----
> BuildIndexInfo(Relation index)
> {
>         IndexInfo  *ii;
>         Form_pg_index indexStruct = index->rd_index;
>         int                     i;
>         int                     numAtts;
>
>         /* check the number of keys, and copy attr numbers into the IndexInfo */
>         numAtts = indexStruct->indnatts;
> ----
>
> And the patch do not have NULL-check for index->rd_index too.
> So I thought we can assume index->rd_index is not null, but it seems I may missed something ?
>
> Can you please share the test case with me ?
>
> I use the following code to replace the call of BuildIndexInfo.
> And the installcheck passed.
>
> Example:
> +                Form_pg_index indexStruct = index_rel->rd_index;
> +                List *ii_Expressions = RelationGetIndexExpressions(index_rel);
> +                int ii_NumIndexAttrs = indexStruct->indnatts;
> +                AttrNumber      ii_IndexAttrNumbers[INDEX_MAX_KEYS];
>
> +                for (int i = 0; i < ii_NumIndexAttrs; i++)
> +                        ii_IndexAttrNumbers[i] = indexStruct->indkey.values[i];

Sorry, I was looking at rel->rd_index, not index_rel->rd_index, my fault.
Your code looks OK. I've taken it and reduced some of the lines and
got rid of the C99-only intermingled variable declarations (see
https://www.postgresql.org/docs/13/source-conventions.html).
The changes are below.
The regression tests all pass, so should be OK (my test case was taken
from insert_parallel regression tests).
Thanks for your help.

-        Oid         index_oid = lfirst_oid(lc);
-        Relation    index_rel;
-        IndexInfo  *index_info;
+        Relation        index_rel;
+        Form_pg_index   indexStruct;
+        List            *ii_Expressions;
+        Oid             index_oid = lfirst_oid(lc);

         index_rel = index_open(index_oid, lockmode);

-        index_info = BuildIndexInfo(index_rel);
+        indexStruct = index_rel->rd_index;
+        ii_Expressions = RelationGetIndexExpressions(index_rel);

-        if (index_info->ii_Expressions != NIL)
+        if (ii_Expressions != NIL)
         {
             int         i;
-            ListCell   *index_expr_item =
list_head(index_info->ii_Expressions);
+            ListCell    *index_expr_item = list_head(ii_Expressions);

-            for (i = 0; i < index_info->ii_NumIndexAttrs; i++)
+            for (i = 0; i < indexStruct->indnatts; i++)
             {
-                int         keycol = index_info->ii_IndexAttrNumbers[i];
+                int         keycol = indexStruct->indkey.values[i];

                 if (keycol == 0)
                 {
@@ -912,7 +914,7 @@ index_expr_max_parallel_hazard_for_modify(Relation rel,
                         return true;
                     }

-                    index_expr_item =
lnext(index_info->ii_Expressions, index_expr_item);
+                    index_expr_item = lnext(ii_Expressions, index_expr_item);
                 }
             }


Regards,
Greg Nancarrow
Fujitsu Australia