Re: BUG #16500: SQL Abend. select multi_key_columns_range_partition_table

> On Wed, Jul 15, 2020 at 04:35:52PM +0900, Etsuro Fujita wrote:
> On Sun, Jul 12, 2020 at 9:55 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > Attached is an updated version of the patch.
>
> Here is a new version of the patch.
>
> Changes:
> * Fix a typo in a comment in get_steps_using_prefix_recurse()
> * Remove redundant regression test cases
> * Add a regression test comment
> * Add the commit message
>
> If there are no objections, I will commit this version.

Thanks for the patch! Sorry, forgot to follow up in time. To the best of
my knowledge the patch looks good, I have just a couple of questions:

+            List       *step_exprs1,
+                       *step_cmpfns1;

             pc = lfirst(lc);
             if (pc->keyno == cur_keyno)
             {
-                /* clean up before starting a new recursion cycle. */
-                if (cur_keyno == 0)
-                {
-                    list_free(step_exprs);
-                    list_free(step_cmpfns);
-                    step_exprs = list_make1(pc->expr);
-                    step_cmpfns = list_make1_oid(pc->cmpfn);
-                }
-                else
-                {
-                    step_exprs = lappend(step_exprs, pc->expr);
-                    step_cmpfns = lappend_oid(step_cmpfns, pc->cmpfn);
-                }
+                /* Leave the original step_exprs unmodified. */
+                step_exprs1 = list_copy(step_exprs);
+                step_exprs1 = lappend(step_exprs1, pc->expr);
+
+                /* Leave the original step_cmpfns unmodified. */
+                step_cmpfns1 = list_copy(step_cmpfns);
+                step_cmpfns1 = lappend_oid(step_cmpfns1, pc->cmpfn);

Does this also belong to some of mentioned fixes? I'm curious if there
could be memory concumption concerns, since it's done within a recursive
function.

-        Assert(list_length(step_exprs) == cur_keyno);
+        Assert(list_length(step_exprs) == cur_keyno ||
+               (context->rel->part_scheme->strategy ==
+                PARTITION_STRATEGY_HASH &&
+                step_opstrategy == HTEqualStrategyNumber &&
+                !bms_is_empty(step_nullkeys) &&
+                bms_num_members(step_nullkeys) + list_length(step_exprs) + 2 ==
+                context->rel->part_scheme->partnatts));

I see the explanation in the commit message, but this assert condition
is quite complex, maybe worth couple of commentary lines right there?