From ed445757b9f4f20ae32afb58e5d207d196c63b54 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 10 Jan 2023 14:15:15 -0500 Subject: [PATCH v7 5/6] Add heapgettup_advance_block() helper --- src/backend/access/heap/heapam.c | 183 +++++++++++++++---------------- 1 file changed, 88 insertions(+), 95 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 223bb1d5d3..4ff3dc7138 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -622,6 +622,90 @@ heapgettup_continue_page(HeapScanDesc scan, ScanDirection dir, int *linesleft, return page; } + +/* + * heapgettup_advance_block - helper for heapgettup() and heapgettup_pagemode() + * + * Given the current block number, the scan direction, and various information + * contained in the scan descriptor, calculate which block to scan next and + * return it. + * + * This should not be called to determine the initial block number -- only for + * subsequent blocks. + * + * Note that this helper may modify scan->rs_numblocks and any state updated by + * table_block_parallelscan_nextpage(). + * + */ +static inline BlockNumber +heapgettup_advance_block(HeapScanDesc scan, BlockNumber block, ScanDirection dir) +{ + if (ScanDirectionIsBackward(dir)) + { + if (block == scan->rs_startblock) + return InvalidBlockNumber; + + if (scan->rs_numblocks != InvalidBlockNumber) + { + if (--scan->rs_numblocks == 0) + return InvalidBlockNumber; + } + + if (block == 0) + block = scan->rs_nblocks; + + block--; + + return block; + } + else if (scan->rs_base.rs_parallel != NULL) + { + Assert(ScanDirectionIsForward(dir)); + + block = table_block_parallelscan_nextpage(scan->rs_base.rs_rd, + scan->rs_parallelworkerdata, (ParallelBlockTableScanDesc) + scan->rs_base.rs_parallel); + + return block; + } + else + { + Assert(ScanDirectionIsForward(dir)); + + block++; + + if (block >= scan->rs_nblocks) + block = 0; + + if (block == scan->rs_startblock) + return InvalidBlockNumber; + + if (scan->rs_numblocks != InvalidBlockNumber) + { + if (--scan->rs_numblocks == 0) + return InvalidBlockNumber; + } + + /* + * Report our new scan position for synchronization purposes. We don't + * do that when moving backwards, however. That would just mess up any + * other forward-moving scanners. + * + * Note: we do this before checking for end of scan so that the final + * state of the position hint is back at the start of the rel. That's + * not strictly necessary, but otherwise when you run the same query + * multiple times the starting position would shift a little bit + * backwards on every invocation, which is confusing. We don't + * guarantee any specific ordering in general, though. + */ + if (scan->rs_base.rs_flags & SO_ALLOW_SYNC) + ss_report_location(scan->rs_base.rs_rd, block); + + return block; + } +} + + /* ---------------- * heapgettup - fetch next heap tuple * @@ -654,7 +738,6 @@ heapgettup(HeapScanDesc scan, HeapTuple tuple = &(scan->rs_ctup); bool backward = ScanDirectionIsBackward(dir); BlockNumber block; - bool finished; Page page; OffsetNumber lineoff; int linesleft; @@ -763,56 +846,12 @@ heapgettup(HeapScanDesc scan, */ LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); - /* - * advance to next/prior page and detect end of scan - */ - if (backward) - { - finished = (block == scan->rs_startblock) || - (scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false); - if (block == 0) - block = scan->rs_nblocks; - block--; - } - else if (scan->rs_base.rs_parallel != NULL) - { - ParallelBlockTableScanDesc pbscan = - (ParallelBlockTableScanDesc) scan->rs_base.rs_parallel; - ParallelBlockTableScanWorker pbscanwork = - scan->rs_parallelworkerdata; - - block = table_block_parallelscan_nextpage(scan->rs_base.rs_rd, - pbscanwork, pbscan); - finished = (block == InvalidBlockNumber); - } - else - { - block++; - if (block >= scan->rs_nblocks) - block = 0; - finished = (block == scan->rs_startblock) || - (scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false); - - /* - * Report our new scan position for synchronization purposes. We - * don't do that when moving backwards, however. That would just - * mess up any other forward-moving scanners. - * - * Note: we do this before checking for end of scan so that the - * final state of the position hint is back at the start of the - * rel. That's not strictly necessary, but otherwise when you run - * the same query multiple times the starting position would shift - * a little bit backwards on every invocation, which is confusing. - * We don't guarantee any specific ordering in general, though. - */ - if (scan->rs_base.rs_flags & SO_ALLOW_SYNC) - ss_report_location(scan->rs_base.rs_rd, block); - } + block = heapgettup_advance_block(scan, block, dir); /* * return NULL if we've exhausted all the pages */ - if (finished) + if (block == InvalidBlockNumber) { if (BufferIsValid(scan->rs_cbuf)) ReleaseBuffer(scan->rs_cbuf); @@ -867,7 +906,6 @@ heapgettup_pagemode(HeapScanDesc scan, HeapTuple tuple = &(scan->rs_ctup); bool backward = ScanDirectionIsBackward(dir); BlockNumber block; - bool finished; Page page; int lineindex; OffsetNumber lineoff; @@ -961,57 +999,12 @@ heapgettup_pagemode(HeapScanDesc scan, ++lineindex; } - /* - * if we get here, it means we've exhausted the items on this page and - * it's time to move to the next. - */ - if (backward) - { - finished = (block == scan->rs_startblock) || - (scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false); - if (block == 0) - block = scan->rs_nblocks; - block--; - } - else if (scan->rs_base.rs_parallel != NULL) - { - ParallelBlockTableScanDesc pbscan = - (ParallelBlockTableScanDesc) scan->rs_base.rs_parallel; - ParallelBlockTableScanWorker pbscanwork = - scan->rs_parallelworkerdata; - - block = table_block_parallelscan_nextpage(scan->rs_base.rs_rd, - pbscanwork, pbscan); - finished = (block == InvalidBlockNumber); - } - else - { - block++; - if (block >= scan->rs_nblocks) - block = 0; - finished = (block == scan->rs_startblock) || - (scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false); - - /* - * Report our new scan position for synchronization purposes. We - * don't do that when moving backwards, however. That would just - * mess up any other forward-moving scanners. - * - * Note: we do this before checking for end of scan so that the - * final state of the position hint is back at the start of the - * rel. That's not strictly necessary, but otherwise when you run - * the same query multiple times the starting position would shift - * a little bit backwards on every invocation, which is confusing. - * We don't guarantee any specific ordering in general, though. - */ - if (scan->rs_base.rs_flags & SO_ALLOW_SYNC) - ss_report_location(scan->rs_base.rs_rd, block); - } + block = heapgettup_advance_block(scan, block, dir); /* * return NULL if we've exhausted all the pages */ - if (finished) + if (block == InvalidBlockNumber) { if (BufferIsValid(scan->rs_cbuf)) ReleaseBuffer(scan->rs_cbuf); -- 2.37.2