On Fri, Oct 25, 2024 at 10:29 AM Melanie Plageman
<melanieplage...@gmail.com> wrote:
>
> Tom suggested off-list that if rs_cindex can't be zero, then it should
> be unsigned. I checked the other scan types using the
> HeapScanDescData, and it seems none of them use values of rs_cindex or
> rs_ntuples < 0. As such, here is a patch making both rs_ntuples and
> rs_cindex unsigned.
I realized one of the asserts was superfluous. Please find v3 attached.
- Melanie
From 262984fde17c1ae3ba220eb5533c8598fb55cae1 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Fri, 25 Oct 2024 09:09:24 -0400
Subject: [PATCH v3] Make rs_cindex and rs_ntuples unsigned
HeapScanDescData.rs_cindex and rs_ntuples can't be less than 0. All scan
types using the heap scan descriptor expect these values to be >= 0.
Make that expectation clear by making rs_cindex and rs_ntuples unsigned.
Also remove the test in heapam_scan_bitmap_next_tuple() that checks if
rs_cindex < 0. This was never true, but now that rs_cindex is unsigned,
it makes even less sense.
While we are at it, initialize both rs_cindex and rs_ntuples to 0 in
initscan().
Author: Melanie Plageman
---
src/backend/access/heap/heapam.c | 7 +++++--
src/backend/access/heap/heapam_handler.c | 2 +-
src/include/access/heapam.h | 4 ++--
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 4c8febdc811..194f64c540d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -378,6 +378,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
ItemPointerSetInvalid(&scan->rs_ctup.t_self);
scan->rs_cbuf = InvalidBuffer;
scan->rs_cblock = InvalidBlockNumber;
+ scan->rs_ntuples = 0;
+ scan->rs_cindex = 0;
/*
* Initialize to ForwardScanDirection because it is most common and
@@ -943,8 +945,8 @@ heapgettup_pagemode(HeapScanDesc scan,
{
HeapTuple tuple = &(scan->rs_ctup);
Page page;
- int lineindex;
- int linesleft;
+ uint32 lineindex;
+ uint32 linesleft;
if (likely(scan->rs_inited))
{
@@ -989,6 +991,7 @@ continue_page:
ItemId lpp;
OffsetNumber lineoff;
+ Assert(lineindex <= scan->rs_ntuples);
lineoff = scan->rs_vistuples[lineindex];
lpp = PageGetItemId(page, lineoff);
Assert(ItemIdIsNormal(lpp));
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 8c59b77b64f..82668fab19a 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2269,7 +2269,7 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
/*
* Out of range? If so, nothing more to look at on this page
*/
- if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)
+ if (hscan->rs_cindex >= hscan->rs_ntuples)
return false;
targoffset = hscan->rs_vistuples[hscan->rs_cindex];
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index b951466ced2..21c7b70edf2 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -103,8 +103,8 @@ typedef struct HeapScanDescData
int rs_empty_tuples_pending;
/* these fields only used in page-at-a-time mode and for bitmap scans */
- int rs_cindex; /* current tuple's index in vistuples */
- int rs_ntuples; /* number of visible tuples on page */
+ uint32 rs_cindex; /* current tuple's index in vistuples */
+ uint32 rs_ntuples; /* number of visible tuples on page */
OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */
} HeapScanDescData;
typedef struct HeapScanDescData *HeapScanDesc;
--
2.34.1
