Re: [HACKERS] PATCH: multivariate histograms and MCV lists

On Wed, 13 Mar 2019 at 17:20, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> bms_member_index seems working differently than maybe expected.
>
>  bms_member_index((2, 4), 0) => 0, (I think) should be -1
>  bms_member_index((2, 4), 1) => 0, should be -1
>  bms_member_index((2, 4), 2) => 0, should be 0
>  bms_member_index((2, 4), 3) => 1, should be -1
>  bms_member_index((2, 4), 4) => 1, should be 1
>  bms_member_index((2, 4), 5) => 2, should be -1
>  bms_member_index((2, 4), 6) => 2, should be -1
> ...
>  bms_member_index((2, 4), 63) => 2, should be -1
>  bms_member_index((2, 4), 64) => -1, correct
>
> It works correctly only when x is a member - the way the function
> is maybe actually used in this patch -, or needs to change the
> specifiction (or the comment) of the function.

Looks like:

+ if (wordnum >= a->nwords)
+ return -1;

should be:

+ if (wordnum >= a->nwords ||
+ (a->word[wordnum] & ((bitmapword) 1 << bitnum)) == 0)
+ return -1;

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services