Re: Report error position in partition bound check

Hi Ashutosh,

I had forgotten about this thread, but Michael's ping email brought it
to my attention.

On Fri, Sep 4, 2020 at 11:12 PM Ashutosh Bapat
<ashutosh.bapat@2ndquadrant.com> wrote:
> Thanks for rebasing patch. It applies cleanly still. Here are some comments

Thanks for the review.

> @@ -3320,7 +3338,9 @@ make_one_partition_rbound(PartitionKey key, int index, List *datums, bool lower)
>   * partition_rbound_cmp
>   *
>   * Return for two range bounds whether the 1st one (specified in datums1,
>
> I think it's better to reword it as. "For two range bounds decide whether ...
>
> - * kind1, and lower1) is <, =, or > the bound specified in *b2.
> + * kind1, and lower1) is <, =, or > the bound specified in *b2. 0 is returned if
> + * equal and the 1-based index of the first mismatching bound if unequal;
> + * multiplied by -1 if the 1st bound is smaller.
>
> This sentence makes sense after the above correction. I liked this change,
> requires very small changes in other parts.

+1 to your suggested rewording, although I wrote: "For two range
bounds this decides whether..."

>  /*
> @@ -3495,7 +3518,7 @@ static int
>  partition_range_bsearch(int partnatts, FmgrInfo *partsupfunc,
>                         Oid *partcollation,
>                         PartitionBoundInfo boundinfo,
> -                       PartitionRangeBound *probe, bool *is_equal)
> +                       PartitionRangeBound *probe, bool *is_equal, int32 *cmpval)
>
> Please update the prologue explaining the new argument.

Done.  Actually, I noticed that *is_equal was unused in this
function's only caller.  *cmpval == 0 already gives that, so removed
is_equal parameter.

Attached updated version.

-- 
Amit Langote
EnterpriseDB: http://www.enterprisedb.com