From 032a5b44d2ceb931ad3303e3be3967ab2212d56c Mon Sep 17 00:00:00 2001 From: Ewan Young Date: Thu, 17 Sep 2026 22:58:12 +0800 Subject: [PATCH] Fix TID range scans that change direction mid-scan heap_setscanlimits() stores the number of blocks to scan in rs_numblocks, and heapgettup_advance_block() treated that as a remaining budget, decrementing it once per block handed out in either direction and ending the scan when it hit zero. That only works for a scan that runs in one direction. A scrollable cursor over a TID Range Scan that moves forward and then backward kept spending the same budget, so the backward leg stopped early and rows in range were never returned; a later forward leg was cut short as well. Treat the limit as a fixed window of rs_numblocks blocks starting at rs_startblock instead: a forward scan ends when it reaches the block just past the window, and a backward scan ends at rs_startblock, which the existing check already handled since heapgettup_initial_block() starts a backward scan at the window's last block. rs_numblocks is no longer modified during the scan. TID Range Scans are the only caller that can scan a limited range in both directions, so this only affects them. Reported-by: Jeff Davis Discussion: https://postgr.es/m/6b50233fcc0f7d6f4845f083777fa9702d9612e4.camel@j-davis.com --- src/backend/access/heap/heapam.c | 28 +++++--- src/test/regress/expected/tidrangescan.out | 75 ++++++++++++++++++++++ src/test/regress/sql/tidrangescan.sql | 10 +++ 3 files changed, 103 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 9ebb1b35d37..e35057e6cc0 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -908,10 +908,20 @@ heapgettup_advance_block(HeapScanDesc scan, BlockNumber block, ScanDirection dir if (block == scan->rs_startblock) return InvalidBlockNumber; - /* check if the limit imposed by heap_setscanlimits() is met */ + /* + * Check if the limit imposed by heap_setscanlimits() is met. The + * limit is a fixed window of rs_numblocks blocks starting at + * rs_startblock, so compare against its end rather than counting + * down a budget; a scan that changes direction (a SCROLL cursor) + * would otherwise exhaust the budget and stop early. + */ if (scan->rs_numblocks != InvalidBlockNumber) { - if (--scan->rs_numblocks == 0) + BlockNumber endblock; + + endblock = (scan->rs_startblock + scan->rs_numblocks) % + scan->rs_nblocks; + if (block == endblock) return InvalidBlockNumber; } @@ -919,17 +929,15 @@ heapgettup_advance_block(HeapScanDesc scan, BlockNumber block, ScanDirection dir } else { - /* we're done if the last block is the start position */ + /* + * We're done if the last block is the start position. This also + * covers the limit imposed by heap_setscanlimits(), since the window + * begins at rs_startblock and heapgettup_initial_block() started us + * at its last block. + */ if (block == scan->rs_startblock) return InvalidBlockNumber; - /* check if the limit imposed by heap_setscanlimits() is met */ - if (scan->rs_numblocks != InvalidBlockNumber) - { - if (--scan->rs_numblocks == 0) - return InvalidBlockNumber; - } - /* wrap to the end of the heap when the last page was page 0 */ if (block == 0) block = scan->rs_nblocks; diff --git a/src/test/regress/expected/tidrangescan.out b/src/test/regress/expected/tidrangescan.out index 89930f7bc7c..e567edfbff8 100644 --- a/src/test/regress/expected/tidrangescan.out +++ b/src/test/regress/expected/tidrangescan.out @@ -295,6 +295,81 @@ FETCH LAST c; (0,10) (1 row) +COMMIT; +-- a scrollable cursor that changes direction mid-scan must still stay within +-- the block range computed by heap_setscanlimits() +BEGIN; +DECLARE c SCROLL CURSOR FOR SELECT ctid FROM tidrangescan WHERE ctid >= '(0,1)' AND ctid <= '(2,10)'; +MOVE FORWARD 25 c; +FETCH BACKWARD 2 c; + ctid +------- + (2,4) + (2,3) +(2 rows) + +FETCH BACKWARD ALL c; + ctid +-------- + (2,2) + (2,1) + (1,10) + (1,9) + (1,8) + (1,7) + (1,6) + (1,5) + (1,4) + (1,3) + (1,2) + (1,1) + (0,10) + (0,9) + (0,8) + (0,7) + (0,6) + (0,5) + (0,4) + (0,3) + (0,2) + (0,1) +(22 rows) + +FETCH FORWARD ALL c; + ctid +-------- + (0,1) + (0,2) + (0,3) + (0,4) + (0,5) + (0,6) + (0,7) + (0,8) + (0,9) + (0,10) + (1,1) + (1,2) + (1,3) + (1,4) + (1,5) + (1,6) + (1,7) + (1,8) + (1,9) + (1,10) + (2,1) + (2,2) + (2,3) + (2,4) + (2,5) + (2,6) + (2,7) + (2,8) + (2,9) + (2,10) +(30 rows) + COMMIT; DROP TABLE tidrangescan; -- Tests for parallel TID Range Scans diff --git a/src/test/regress/sql/tidrangescan.sql b/src/test/regress/sql/tidrangescan.sql index 1ac3995e71c..55e266c431c 100644 --- a/src/test/regress/sql/tidrangescan.sql +++ b/src/test/regress/sql/tidrangescan.sql @@ -96,6 +96,16 @@ FETCH FIRST c; FETCH LAST c; COMMIT; +-- a scrollable cursor that changes direction mid-scan must still stay within +-- the block range computed by heap_setscanlimits() +BEGIN; +DECLARE c SCROLL CURSOR FOR SELECT ctid FROM tidrangescan WHERE ctid >= '(0,1)' AND ctid <= '(2,10)'; +MOVE FORWARD 25 c; +FETCH BACKWARD 2 c; +FETCH BACKWARD ALL c; +FETCH FORWARD ALL c; +COMMIT; + DROP TABLE tidrangescan; -- Tests for parallel TID Range Scans -- 2.47.3