From 1b56abd762108ab6d50b9f8d7ebd43dbae9329c3 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Tue, 10 Jan 2023 14:15:15 -0500 Subject: [PATCH v5 6/7] Add heapgettup_advance_block() helper --- src/backend/access/heap/heapam.c | 167 +++++++++++++------------------ 1 file changed, 72 insertions(+), 95 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index bd3bc3e52f..65c05d1b15 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -630,6 +630,74 @@ heapgettup_continue_page(HeapScanDesc scan, BlockNumber block, ScanDirection return page; } +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 * @@ -662,7 +730,6 @@ heapgettup(HeapScanDesc scan, HeapTuple tuple = &(scan->rs_ctup); bool backward = ScanDirectionIsBackward(dir); BlockNumber block; - bool finished; Page page; OffsetNumber lineoff; int linesleft; @@ -769,56 +836,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); @@ -873,7 +896,6 @@ heapgettup_pagemode(HeapScanDesc scan, HeapTuple tuple = &(scan->rs_ctup); bool backward = ScanDirectionIsBackward(dir); BlockNumber block; - bool finished; Page page; int lineindex; OffsetNumber lineoff; @@ -965,57 +987,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.34.1