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

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

> On Wed, Dec 18, 2024 at 3:13 PM Melanie Plageman
> <melanieplageman(at)gmail(dot)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
>