Re: Can rs_cindex be < 0 for bitmap heap scans?

Hi.

Em qua., 18 de dez. de 2024 às 14:01, Melanie Plageman <
melanieplageman(at)gmail(dot)com> escreveu:

> On Mon, Oct 28, 2024 at 2:27 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Sat, Oct 26, 2024 at 5:30 PM Melanie Plageman <
> melanieplageman(at)gmail(dot)com> wrote:
> >>
> >> Yes, so in the case of backwards scan, if scan->rs_cindex is 0, when
> >> dir is -1, lineindex will wrap around, but we don't use it in that
> >> case because linesleft will be 0 and then we will not meet the
> >> conditions to execute the code in the loop under continue_page. And in
> >> the loop, when adding -1 to lineindex, for backwards scan, it always
> >> starts at linesleft and ticks down to 0.
> >
> >
> > Yeah you are right, although the lineindex will wrap around when
> rs_cindex is 0 , it would not be used. So, it won’t actually cause any
> issues, but I’m not comfortable with the unintentional wraparound. I would
> have left "scan->rs_cindex" as int itself but I am fine with whatever you
> prefer.
>
> So, I don't see -1 as different from the wrapped around value with an
> unsigned data type. Also neither is a valid number of entries in
> rs_vistuples (which is limited to MaxHeapTuplesPerPage).
>
> That being said, I can see how having lineindex be an invalid value
> could be confusing -- either with an unsigned or signed data type for
> rs_cindex. I think to make this clear we would have to add another
> special case for backwards and no movement scan.
>
> For now, I've committed the version of the patch I proposed above (v3).
>
What happens if *rs_tuples* equal to zero in function
*SampleHeapTupleVisible*?
end = hscan->rs_ntuples - 1;

Would be good to fix the binary search too, now that unsigned types are
used.

Patch attached.

best regards,
Ranier Vilela