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

On Wed, Dec 18, 2024 at 9:50 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
>
> On Thu, Dec 19, 2024 at 8:18 AM Melanie Plageman
> <melanieplageman(at)gmail(dot)com> wrote:
> > I pushed the straightforward option for now so that it's fixed.
>
> I think this binary search code now has a risk of underflow. If 'mid'
> is calculated as zero, the second 'if' branch will cause 'end' to
> underflow.

Thanks, Richard!

> Maybe we need to do something like below.
>
> --- a/src/backend/access/heap/heapam_handler.c
> +++ b/src/backend/access/heap/heapam_handler.c
> @@ -2600,7 +2600,11 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
> if (tupoffset == curoffset)
> return true;
> else if (tupoffset < curoffset)
> + {
> + if (mid == 0)
> + return false;
> end = mid - 1;
> + }
> else
> start = mid + 1;
> }
>
> Alternatively, we can revert 'start' and 'end' to signed int as they
> were before.

What about this instead:

diff --git a/src/backend/access/heap/heapam_handler.c
b/src/backend/access/heap/heapam_handler.c
index 2da4e4da13e..fb90fd869c2 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2574,11 +2574,8 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,

if (scan->rs_flags & SO_ALLOW_PAGEMODE)
{
- uint32 start,
- end;
-
- if (hscan->rs_ntuples == 0)
- return false;
+ uint32 start = 0,
+ end = hscan->rs_ntuples;

/*
* In pageatatime mode, heap_prepare_pagescan() already did visibility
@@ -2589,10 +2586,8 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
* in increasing order, but it's not clear that there would be enough
* gain to justify the restriction.
*/
- start = 0;
- end = hscan->rs_ntuples - 1;

- while (start <= end)
+ while (start < end)
{
uint32 mid = (start + end) / 2;
OffsetNumber curoffset = hscan->rs_vistuples[mid];
@@ -2600,7 +2595,7 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
if (tupoffset == curoffset)
return true;
else if (tupoffset < curoffset)
- end = mid - 1;
+ end = mid;
else
start = mid + 1;
}

Or to make it easier to read, here:

uint32 start = 0,
end = hscan->rs_ntuples;

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

if (tupoffset == curoffset)
return true;
else if (tupoffset < curoffset)
end = mid;
else
start = mid + 1;
}

I think this gets rid of any subtraction and is still the same.

- Melanie