Re: Can rs_cindex be < 0 for bitmap heap scans? - Mailing list pgsql-hackers

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.

- Melanie