From 32652b312d0c802e3105c064c24ff5b99694399c Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 2 Feb 2023 15:07:27 +1300 Subject: [PATCH v9] Further refactor of heapgettup and heapgettup_pagemode Backward and forward scans share much of the same page acquisition code. Here we consolidate that code to reduce some duplication. Additionally, add a new rs_coffset field to HeapScanDescData to track the offset of the current tuple. The new field fits nicely into the padding between a bool and BlockNumber field and saves having to look at the last returned tuple to figure out which offset we should be looking at for the current tuple. Author: Melanie Plageman Reviewed-by: David Rowley Discussion: https://postgr.es/m/CAAKRu_bvkhka0CZQun28KTqhuUh5ZqY=_T8QEqZqOL02rpi2bw@mail.gmail.com --- src/backend/access/heap/heapam.c | 200 ++++++++++--------------------- src/include/access/heapam.h | 1 + 2 files changed, 63 insertions(+), 138 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index b34e20a71f..ec6b7962c5 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -576,101 +576,67 @@ heapgettup(HeapScanDesc scan, BlockNumber block; bool finished; Page page; - int lines; OffsetNumber lineoff; int linesleft; ItemId lpp; - /* - * calculate next starting lineoff, given scan direction - */ - if (ScanDirectionIsForward(dir)) + if (unlikely(!scan->rs_inited)) { - if (!scan->rs_inited) - { - block = heapgettup_initial_block(scan, dir); + block = heapgettup_initial_block(scan, dir); - /* - * Check if we have reached the end of the scan already. This - * could happen if the table is empty or if the parallel workers - * have already finished the scan before we did anything ourselves - */ - if (block == InvalidBlockNumber) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } - heapgetpage((TableScanDesc) scan, block); - lineoff = FirstOffsetNumber; /* first offnum */ - scan->rs_inited = true; - } - else + /* + * Check if we have reached the end of the scan already. This could + * happen if the table is empty or if the parallel workers have + * already finished the scan before we did anything ourselves + */ + if (block == InvalidBlockNumber) { - /* continue from previously returned page/tuple */ - block = scan->rs_cblock; /* current page */ - lineoff = /* next offnum */ - OffsetNumberNext(ItemPointerGetOffsetNumber(&(tuple->t_self))); + Assert(!BufferIsValid(scan->rs_cbuf)); + tuple->t_data = NULL; + return; } - LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE); + heapgetpage((TableScanDesc) scan, block); + LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE); page = BufferGetPage(scan->rs_cbuf); TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page); - lines = PageGetMaxOffsetNumber(page); - /* block and lineoff now reference the physically next tid */ - linesleft = lines - lineoff + 1; + linesleft = PageGetMaxOffsetNumber(page) - FirstOffsetNumber + 1; + + if (ScanDirectionIsForward(dir)) + lineoff = FirstOffsetNumber; /* first offnum */ + else + lineoff = (OffsetNumber) linesleft; + + scan->rs_inited = true; } else { - if (!scan->rs_inited) - { - block = heapgettup_initial_block(scan, dir); - - /* - * Check if we have reached the end of the scan already. This - * could happen if the table is empty. - */ - if (block == InvalidBlockNumber) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } + /* continue from previously returned page/tuple */ + block = scan->rs_cblock; - heapgetpage((TableScanDesc) scan, block); - LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE); + LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE); + page = BufferGetPage(scan->rs_cbuf); + TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page); - page = BufferGetPage(scan->rs_cbuf); - TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page); - lines = PageGetMaxOffsetNumber(page); - lineoff = lines; /* final offnum */ - scan->rs_inited = true; + if (ScanDirectionIsForward(dir)) + { + lineoff = OffsetNumberNext(scan->rs_coffset); + linesleft = PageGetMaxOffsetNumber(page) - lineoff + 1; } else { - /* continue from previously returned page/tuple */ - block = scan->rs_cblock; /* current page */ - LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE); - - page = BufferGetPage(scan->rs_cbuf); - TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page); - lines = PageGetMaxOffsetNumber(page); - /* * The previous returned tuple may have been vacuumed since the * previous scan when we use a non-MVCC snapshot, so we must * re-establish the lineoff <= PageGetMaxOffsetNumber(page) * invariant */ - lineoff = /* previous offnum */ - Min(lines, - OffsetNumberPrev(ItemPointerGetOffsetNumber(&(tuple->t_self)))); + lineoff = Min(PageGetMaxOffsetNumber(page), + OffsetNumberPrev(scan->rs_coffset)); + linesleft = lineoff; } - /* block and lineoff now reference the physically previous tid */ - - linesleft = lineoff; } /* @@ -715,6 +681,7 @@ heapgettup(HeapScanDesc scan, if (valid) { LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); + scan->rs_coffset = lineoff; return; } } @@ -807,12 +774,11 @@ heapgettup(HeapScanDesc scan, page = BufferGetPage(scan->rs_cbuf); TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page); - lines = PageGetMaxOffsetNumber(page); - linesleft = lines; + linesleft = PageGetMaxOffsetNumber(page); if (backward) { - lineoff = lines; - lpp = PageGetItemId(page, lines); + lineoff = linesleft; + lpp = PageGetItemId(page, linesleft); } else { @@ -846,87 +812,46 @@ heapgettup_pagemode(HeapScanDesc scan, BlockNumber block; bool finished; Page page; - int lines; int lineindex; OffsetNumber lineoff; int linesleft; ItemId lpp; - /* - * calculate next starting lineindex, given scan direction - */ - if (ScanDirectionIsForward(dir)) + if (unlikely(!scan->rs_inited)) { - if (!scan->rs_inited) - { - block = heapgettup_initial_block(scan, dir); + block = heapgettup_initial_block(scan, dir); - /* - * Check if we have reached the end of the scan already. This - * could happen if the table is empty or if the parallel workers - * have already finished the scan before we did anything ourselves - */ - if (block == InvalidBlockNumber) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } - heapgetpage((TableScanDesc) scan, block); - lineindex = 0; - scan->rs_inited = true; - } - else + /* + * Check if we have reached the end of the scan already. This could + * happen if the table is empty or if the other workers in a parallel + * scan have already finished the scan. + */ + if (block == InvalidBlockNumber) { - /* continue from previously returned page/tuple */ - block = scan->rs_cblock; /* current page */ - lineindex = scan->rs_cindex + 1; + Assert(!BufferIsValid(scan->rs_cbuf)); + tuple->t_data = NULL; + return; } + heapgetpage((TableScanDesc) scan, block); page = BufferGetPage(scan->rs_cbuf); TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page); - lines = scan->rs_ntuples; - /* block and lineindex now reference the next visible tid */ - - linesleft = lines - lineindex; + linesleft = scan->rs_ntuples; + lineindex = ScanDirectionIsForward(dir) ? 0 : linesleft - 1; + scan->rs_inited = true; } else { - if (!scan->rs_inited) - { - block = heapgettup_initial_block(scan, dir); - - /* - * Check if we have reached the end of the scan already. This - * could happen if the table is empty. - */ - if (block == InvalidBlockNumber) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } + /* continue from previously returned page/tuple */ + block = scan->rs_cblock; /* current page */ + page = BufferGetPage(scan->rs_cbuf); + TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page); - heapgetpage((TableScanDesc) scan, block); - page = BufferGetPage(scan->rs_cbuf); - TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page); - lines = scan->rs_ntuples; - lineindex = lines - 1; - scan->rs_inited = true; - } + lineindex = scan->rs_cindex + dir; + if (ScanDirectionIsForward(dir)) + linesleft = scan->rs_ntuples - lineindex; else - { - /* continue from previously returned page/tuple */ - block = scan->rs_cblock; /* current page */ - - page = BufferGetPage(scan->rs_cbuf); - TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page); - lines = scan->rs_ntuples; - lineindex = scan->rs_cindex - 1; - } - /* block and lineindex now reference the previous visible tid */ - - linesleft = lineindex + 1; + linesleft = scan->rs_cindex; } /* @@ -1041,10 +966,9 @@ heapgettup_pagemode(HeapScanDesc scan, page = BufferGetPage(scan->rs_cbuf); TestForOldSnapshot(scan->rs_base.rs_snapshot, scan->rs_base.rs_rd, page); - lines = scan->rs_ntuples; - linesleft = lines; + linesleft = scan->rs_ntuples; if (backward) - lineindex = lines - 1; + lineindex = linesleft - 1; else lineindex = 0; } diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index 417108f1e0..8d28bc93ef 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -57,6 +57,7 @@ typedef struct HeapScanDescData /* scan current state */ bool rs_inited; /* false = scan not init'd yet */ + OffsetNumber rs_coffset; /* current offset # in non-page-at-a-time mode */ BlockNumber rs_cblock; /* current block # in scan, if any */ Buffer rs_cbuf; /* current buffer in scan, if any */ /* NB: if rs_cbuf is not InvalidBuffer, we hold a pin on that buffer */ -- 2.39.0.windows.1