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

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b34e20a71ff0e69b075e7f9e2b1ea6c2a3ad52bd..ec6b7962c50f848c2aac41495d1a3ff482fc7db0 100644 (file)
--- 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 417108f1e01c7d4bd407e7d4b3547399de1f71c4..8d28bc93ef4bfc755cead35593f9e73451733b9d 100644 (file)
--- 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 */