postgres.git / summary / log / commit / refs

commit    c84dd31bd9f5c2ba1ca79615d405ea0bf00c51a2
Author:   David Rowley <drowley@postgresql.org>
Date:     Wed Sep 23 13:21:45 2026 +0000

    Fix incorrect block accounting in TID Range Scans
    
    heapgettup_advance_block() (or heapgettup() and heapgettup_pagemode() in
    versions before v16) would incorrectly decrement rs_numblocks when
    advancing to the next block.  Ordinarily this was fine, but if the TID
    Range scan plan was used by a scrollable cursor and the cursor was fetched
    backwards enough to result in another block being required, then
    rs_numblocks may have reached 0 prematurely resulting in the scan ending
    prematurely.
    
    Reported-by: Jeff Davis <pgsql@j-davis.com>
    Author: Ewan Young <kdbase.hack@gmail.com>
    Discussion: https://postgr.es/m/6b50233fcc0f7d6f4845f083777fa9702d9612e4.camel@j-davis.com
    Backpatch-through: 14


 src/backend/access/heap/heapam.c           | 48 +++++++++++++++++++++++++-----
 src/test/regress/expected/tidrangescan.out | 26 ++++++++++++++++
 src/test/regress/sql/tidrangescan.sql      | 12 ++++++++
 3 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a2c2745d148..0bb9cbc48a6 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -767,8 +767,14 @@ heapgettup(HeapScanDesc scan,
 		 */
 		if (backward)
 		{
-			finished = (page == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
+			/*
+			 * We're done if the last block is the start position.  No need to
+			 * check if rs_numblocks was set by heap_setscanlimits() as that
+			 * only changes the end block.  The start block is the same with
+			 * or without scan limits.
+			 */
+			finished = (page == scan->rs_startblock);
+
 			if (page == 0)
 				page = scan->rs_nblocks;
 			page--;
@@ -789,8 +795,19 @@ heapgettup(HeapScanDesc scan,
 			page++;
 			if (page >= scan->rs_nblocks)
 				page = 0;
-			finished = (page == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
+
+			if (page == scan->rs_startblock)
+				finished = true;
+			else if (scan->rs_numblocks != InvalidBlockNumber)
+			{
+				BlockNumber endblock;
+
+				endblock = (scan->rs_startblock + scan->rs_numblocks) %
+					scan->rs_nblocks;
+				finished = (page == endblock);
+			}
+			else
+				finished = false;
 
 			/*
 			 * Report our new scan position for synchronization purposes. We
@@ -1076,8 +1093,13 @@ heapgettup_pagemode(HeapScanDesc scan,
 		 */
 		if (backward)
 		{
-			finished = (page == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
+			/*
+			 * We're done if the last block is the start position.  No need to
+			 * check if rs_numblocks was set by heap_setscanlimits() as that
+			 * only changes the end block.  The start block is the same with
+			 * or without scan limits.
+			 */
+			finished = (page == scan->rs_startblock);
 			if (page == 0)
 				page = scan->rs_nblocks;
 			page--;
@@ -1098,8 +1120,18 @@ heapgettup_pagemode(HeapScanDesc scan,
 			page++;
 			if (page >= scan->rs_nblocks)
 				page = 0;
-			finished = (page == scan->rs_startblock) ||
-				(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);
+			if (page == scan->rs_startblock)
+				finished = true;
+			else if (scan->rs_numblocks != InvalidBlockNumber)
+			{
+				BlockNumber endblock;
+
+				endblock = (scan->rs_startblock + scan->rs_numblocks) %
+					scan->rs_nblocks;
+				finished = (page == endblock);
+			}
+			else
+				finished = false;
 
 			/*
 			 * Report our new scan position for synchronization purposes. We
diff --git a/src/test/regress/expected/tidrangescan.out b/src/test/regress/expected/tidrangescan.out
index 721f3b94e04..3ccef0fd0c2 100644
--- a/src/test/regress/expected/tidrangescan.out
+++ b/src/test/regress/expected/tidrangescan.out
@@ -295,6 +295,32 @@ FETCH LAST c;
  (0,10)
 (1 row)
 
+COMMIT;
+-- Ensure TID Range Scans with scrollable cursors count blocks correctly.
+BEGIN;
+DECLARE c SCROLL CURSOR FOR SELECT ctid FROM tidrangescan WHERE ctid >= '(0,1)' AND ctid <= '(2,10)';
+MOVE FORWARD 25 c;
+MOVE BACKWARD 4 c;
+FETCH BACKWARD 1 c;
+  ctid  
+--------
+ (1,10)
+(1 row)
+
+MOVE BACKWARD ALL c;
+FETCH FORWARD 1 c;
+ ctid  
+-------
+ (0,1)
+(1 row)
+
+MOVE FORWARD ALL c;
+FETCH BACKWARD 1 c;
+  ctid  
+--------
+ (2,10)
+(1 row)
+
 COMMIT;
 DROP TABLE tidrangescan;
 RESET enable_seqscan;
diff --git a/src/test/regress/sql/tidrangescan.sql b/src/test/regress/sql/tidrangescan.sql
index ac09ebb6262..e367292e885 100644
--- a/src/test/regress/sql/tidrangescan.sql
+++ b/src/test/regress/sql/tidrangescan.sql
@@ -96,6 +96,18 @@ FETCH FIRST c;
 FETCH LAST c;
 COMMIT;
 
+-- Ensure TID Range Scans with scrollable cursors count blocks correctly.
+BEGIN;
+DECLARE c SCROLL CURSOR FOR SELECT ctid FROM tidrangescan WHERE ctid >= '(0,1)' AND ctid <= '(2,10)';
+MOVE FORWARD 25 c;
+MOVE BACKWARD 4 c;
+FETCH BACKWARD 1 c;
+MOVE BACKWARD ALL c;
+FETCH FORWARD 1 c;
+MOVE FORWARD ALL c;
+FETCH BACKWARD 1 c;
+COMMIT;
+
 DROP TABLE tidrangescan;
 
 RESET enable_seqscan;


[parent: 851a5869cd27]