From 6e0825d7e484c33bf0b00c95c80285528540cb0e Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 30 Nov 2022 15:06:34 -0500 Subject: [PATCH v4 5/6] Add heapgettup_advance_page() --- src/backend/access/heap/heapam.c | 168 +++++++++++++------------------ 1 file changed, 72 insertions(+), 96 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index e586dda5aa..39b12b198d 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -634,6 +634,74 @@ heapgettup_continue_page(HeapScanDesc scan, BlockNumber block, ScanDirection return page; } +static inline BlockNumber +heapgettup_advance_page(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 * @@ -667,7 +735,6 @@ heapgettup(HeapScanDesc scan, Snapshot snapshot = scan->rs_base.rs_snapshot; bool backward = ScanDirectionIsBackward(dir); BlockNumber block; - bool finished; Page page; OffsetNumber lineoff; int linesleft; @@ -771,56 +838,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_page(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; @@ -969,57 +991,11 @@ 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_page(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.38.1