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

Em qua., 18 de dez. de 2024 às 20:18, Melanie Plageman <
melanieplageman@gmail.com> escreveu:

> On Wed, Dec 18, 2024 at 3:13 PM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> >
> > option 1:
> > most straightforward fix
> >
> > diff --git a/src/backend/access/heap/heapam_handler.c
> > b/src/backend/access/heap/heapam_handler.c
> > index d0e5922eed7..fa03bedd4b8 100644
> > --- a/src/backend/access/heap/heapam_handler.c
> > +++ b/src/backend/access/heap/heapam_handler.c
> > @@ -2577,6 +2577,11 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer
> buffer,
> >
> >     if (scan->rs_flags & SO_ALLOW_PAGEMODE)
> >     {
> > +       int         start, end;
> > +
> > +       if (hscan->rs_ntuples == 0)
> > +           return false;
> > +
> >         /*
> >          * In pageatatime mode, heap_prepare_pagescan() already did
> visibility
> >          * checks, so just look at the info it left in rs_vistuples[].
> > @@ -2586,8 +2591,8 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer
> buffer,
> >          * in increasing order, but it's not clear that there would be
> enough
> >          * gain to justify the restriction.
> >          */
> > -       int         start = 0,
> > -                   end = hscan->rs_ntuples - 1;
> > +       start = 0;
> > +       end = hscan->rs_ntuples - 1;
> >
> >         while (start <= end)
> >         {
> >
> > option 2:
> > change the binary search code a bit more but avoid the extra branch
> > introduced by option 1.
> >
> > diff --git a/src/backend/access/heap/heapam_handler.c
> > b/src/backend/access/heap/heapam_handler.c
> > index d0e5922eed7..c8e25bdd197 100644
> > --- a/src/backend/access/heap/heapam_handler.c
> > +++ b/src/backend/access/heap/heapam_handler.c
> > @@ -2586,18 +2586,17 @@ SampleHeapTupleVisible(TableScanDesc scan,
> > Buffer buffer,
> >          * in increasing order, but it's not clear that there would be
> enough
> >          * gain to justify the restriction.
> >          */
> > -       int         start = 0,
> > -                   end = hscan->rs_ntuples - 1;
> > +       uint32 start = 0, end = hscan->rs_ntuples;
> >
> > -       while (start <= end)
> > +       while (start < end)
> >         {
> > -           int         mid = (start + end) / 2;
> > +           int mid = (start + end) / 2;
> >             OffsetNumber curoffset = hscan->rs_vistuples[mid];
> >
> >             if (tupoffset == curoffset)
> >                 return true;
> >             else if (tupoffset < curoffset)
> > -               end = mid - 1;
> > +               end = mid;
> >             else
> >                 start = mid + 1;
> >         }
>
> I pushed the straightforward option for now so that it's fixed.
>
How about:

diff --git a/src/backend/access/heap/heapam_handler.c
b/src/backend/access/heap/heapam_handler.c
index 2da4e4da13..1620f0c3db 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2574,7 +2574,7 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer
buffer,

  if (scan->rs_flags & SO_ALLOW_PAGEMODE)
  {
- uint32 start,
+ int64 start,
  end;

  if (hscan->rs_ntuples == 0)
@@ -2594,7 +2594,7 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer
buffer,

  while (start <= end)
  {
- uint32 mid = (start + end) / 2;
+ int64 mid = (start + end) >> 1;
  OffsetNumber curoffset = hscan->rs_vistuples[mid];

  if (tupoffset == curoffset)


best regards
Ranier Vilela

>
> - Melanie
>