From: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Obsolete comment in partbounds.c |
Date: | 2019-10-19 08:56:02 |
Message-ID: | CAPmGK14CbvsNn=ze8Z3v4wD8fgjDpBu8ggzF+_7VMsq9vnrHyg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Alvaro,
On Fri, Oct 18, 2019 at 6:56 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Colin Watson | 2019-10-19 09:22:39 | Re: Backport "WITH ... AS MATERIALIZED" syntax to <12? |
Previous Message | Ariadne Conill | 2019-10-19 06:52:11 | Re: jsonb_set() strictness considered harmful to data |