From 60d11cdba93595301ab1d2f235ca8df7d327cf7c Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Wed, 30 Nov 2022 12:08:05 -0500 Subject: [PATCH v4 3/6] Streamline heapgettup*() for refactor Flip initial logic to set local variables in order to make it easier to add helper functions. Also, remove unneeded local variables. --- src/backend/access/heap/heapam.c | 168 ++++++++++--------------------- src/include/access/heapam.h | 6 +- 2 files changed, 58 insertions(+), 116 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 37795eab67..ed1980dffa 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -610,7 +610,6 @@ heapgettup(HeapScanDesc scan, BlockNumber block; bool finished; Page page; - int lines; OffsetNumber lineoff; int linesleft; ItemId lpp; @@ -621,84 +620,57 @@ heapgettup(HeapScanDesc scan, return; } - if (ScanDirectionIsForward(dir)) + if (!scan->rs_inited) { - if (!scan->rs_inited) - { - block = heapgettup_initial_page(scan, dir); + block = heapgettup_initial_page(scan, dir); - if (block == InvalidBlockNumber) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } - heapgetpage((TableScanDesc) scan, block); - lineoff = FirstOffsetNumber; - } - else + 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) page) - FirstOffsetNumber + 1; + + if (ScanDirectionIsForward(dir)) + lineoff = FirstOffsetNumber; + else + lineoff = (OffsetNumber) linesleft; } else { - Assert(backward); - - if (!scan->rs_inited) - { - block = heapgettup_initial_page(scan, dir); + block = scan->rs_cblock; /* current page */ - if (block == InvalidBlockNumber) - { - 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); - page = BufferGetPage(scan->rs_cbuf); - TestForOldSnapshot(snapshot, scan->rs_base.rs_rd, page); - lines = PageGetMaxOffsetNumber(page); - lineoff = lines; /* final offnum */ + if (ScanDirectionIsForward(dir)) + { + lineoff = OffsetNumberNext(scan->rs_cindex); + /* block and lineoff now reference the physically next tid */ + 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_cindex)); + /* block and lineoff now reference the physically previous tid */ + linesleft = lineoff; } - /* block and lineoff now reference the physically previous tid */ - - linesleft = lineoff; } /* @@ -743,6 +715,7 @@ heapgettup(HeapScanDesc scan, if (valid) { LockBuffer(scan->rs_cbuf, BUFFER_LOCK_UNLOCK); + scan->rs_cindex = lineoff; return; } } @@ -835,12 +808,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 { @@ -874,7 +846,6 @@ heapgettup_pagemode(HeapScanDesc scan, BlockNumber block; bool finished; Page page; - int lines; int lineindex; OffsetNumber lineoff; int linesleft; @@ -886,73 +857,41 @@ heapgettup_pagemode(HeapScanDesc scan, return; } - /* - * calculate next starting lineindex, given scan direction - */ - if (ScanDirectionIsForward(dir)) + if (!scan->rs_inited) { - if (!scan->rs_inited) - { - block = heapgettup_initial_page(scan, dir); + block = heapgettup_initial_page(scan, dir); - if (block == InvalidBlockNumber) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } - - heapgetpage((TableScanDesc) scan, block); - lineindex = 0; - } - else + 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; } else { - Assert(backward); - - if (!scan->rs_inited) - { - block = heapgettup_initial_page(scan, dir); + /* continue from previously returned page/tuple */ + block = scan->rs_cblock; /* current page */ - if (block == InvalidBlockNumber) - { - Assert(!BufferIsValid(scan->rs_cbuf)); - tuple->t_data = NULL; - return; - } + 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; - } + 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; } + /* + * block and lineindex now reference the next or previous visible tid + */ + /* * advance the scan until we find a qualifying tuple or run out of stuff @@ -1066,10 +1005,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..f0bd26897f 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -72,8 +72,12 @@ typedef struct HeapScanDescData */ ParallelBlockTableScanWorkerData *rs_parallelworkerdata; + /* + * current tuple's index in vistuples or current lineoff in page + */ + int rs_cindex; + /* these fields only used in page-at-a-time mode and for bitmap scans */ - int rs_cindex; /* current tuple's index in vistuples */ int rs_ntuples; /* number of visible tuples on page */ OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */ } HeapScanDescData; -- 2.38.1