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

At Wed, 13 Mar 2019 19:37:45 +1300, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote in <CAKJS1f_6qDQj9m2H0jF4bRkZVLpfc7O9E+MxdXrq0wgv0z1NrQ(at)mail(dot)gmail(dot)com>
> On Wed, 13 Mar 2019 at 17:20, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)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;

Yeah, seems right.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center