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?