Re: Obsolete comment in partbounds.c

Hi Alvaro,

On Fri, Oct 18, 2019 at 6:56 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2019-Oct-18, Etsuro Fujita wrote:
> > While reviewing the partitionwise-join patch, I noticed $Subject，ie,
> > this in create_list_bounds():
> >
> >                 /*
> >                  * Never put a null into the values array, flag instead for
> >                  * the code further down below where we construct the actual
> >                  * relcache struct.
> >                  */
> >                 if (null_index != -1)
> >                     elog(ERROR, "found null more than once");
> >                 null_index = i;
> >
> > "the code further down below where we construct the actual relcache
> > struct" isn't in the same file anymore by refactoring by commit
> > b52b7dc25.  How about modifying it like the attached?
>
> Yeah, agreed.  Instead of "the null comes from" I would use "the
> partition that stores nulls".

I think your wording is better than mine.  Thank you for reviewing!

> While reviewing your patch I noticed a few places where we use an odd
> pattern in switches, which can be simplified as shown here.

        case PARTITION_STRATEGY_LIST:
-           num_indexes = bound->ndatums;
+           return bound->ndatums;
            break;

Why not remove the break statement?

Other than that the patch looks good to me.

Best regards,
Etsuro Fujita