Preliminary refactor of heap scanning functions

To allow the use of the read stream API added in b5a9b18cd for
sequential scans on heap tables, here we make some adjustments to make
that change less invasive and perhaps make the code easier to follow in
the process.

Here heapgetpage() gets broken into two functions:

1) The part which reads the block has now been moved into a function
   named heapfetchbuf().
2) The part which performed pruning and populated the scan's
   rs_vistuples[] array is now moved into a new function named
   heap_prepare_pagescan().

The functionality provided by heap_prepare_pagescan() was only ever
required by SO_ALLOW_PAGEMODE scans, so the branching that was
previously done in heapgetpage() is no longer needed as we simply just
don't call heap_prepare_pagescan() from heapgettup() in the refactored
code.

Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_YtXJiYKQvb5JsA2SkwrsizYLugs4sSOZh3EAjKUg=gEQ@mail.gmail.com

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a9d5b109a5ecc71b83b06467855e13cf7971e514..7906d5d7c5bd29001cc14392bb604ecd0cd05b33 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -360,17 +360,17 @@ heap_setscanlimits(TableScanDesc sscan, BlockNumber startBlk, BlockNumber numBlk
 }
 
 /*
- * heapgetpage - subroutine for heapgettup()
+ * heap_prepare_pagescan - Prepare current scan page to be scanned in pagemode
  *
- * This routine reads and pins the specified page of the relation.
- * In page-at-a-time mode it performs additional work, namely determining
- * which tuples on the page are visible.
+ * Preparation currently consists of 1. prune the scan's rs_cbuf page, and 2.
+ * fill the rs_vistuples[] array with the OffsetNumbers of visible tuples.
  */
 void
-heapgetpage(TableScanDesc sscan, BlockNumber block)
+heap_prepare_pagescan(TableScanDesc sscan)
 {
    HeapScanDesc scan = (HeapScanDesc) sscan;
-   Buffer      buffer;
+   Buffer      buffer = scan->rs_cbuf;
+   BlockNumber block = scan->rs_cblock;
    Snapshot    snapshot;
    Page        page;
    int         lines;
@@ -378,31 +378,10 @@ heapgetpage(TableScanDesc sscan, BlockNumber block)
    OffsetNumber lineoff;
    bool        all_visible;
 
-   Assert(block < scan->rs_nblocks);
-
-   /* release previous scan buffer, if any */
-   if (BufferIsValid(scan->rs_cbuf))
-   {
-       ReleaseBuffer(scan->rs_cbuf);
-       scan->rs_cbuf = InvalidBuffer;
-   }
-
-   /*
-    * Be sure to check for interrupts at least once per page.  Checks at
-    * higher code levels won't be able to stop a seqscan that encounters many
-    * pages' worth of consecutive dead tuples.
-    */
-   CHECK_FOR_INTERRUPTS();
-
-   /* read page using selected strategy */
-   scan->rs_cbuf = ReadBufferExtended(scan->rs_base.rs_rd, MAIN_FORKNUM, block,
-                                      RBM_NORMAL, scan->rs_strategy);
-   scan->rs_cblock = block;
-
-   if (!(scan->rs_base.rs_flags & SO_ALLOW_PAGEMODE))
-       return;
+   Assert(BufferGetBlockNumber(buffer) == block);
 
-   buffer = scan->rs_cbuf;
+   /* ensure we're not accidentally being used when not in pagemode */
+   Assert(scan->rs_base.rs_flags & SO_ALLOW_PAGEMODE);
    snapshot = scan->rs_base.rs_snapshot;
 
    /*
@@ -475,6 +454,37 @@ heapgetpage(TableScanDesc sscan, BlockNumber block)
    scan->rs_ntuples = ntup;
 }
 
+/*
+ * heapfetchbuf - read and pin the given MAIN_FORKNUM block number.
+ *
+ * Read the specified block of the scan relation into a buffer and pin that
+ * buffer before saving it in the scan descriptor.
+ */
+static inline void
+heapfetchbuf(HeapScanDesc scan, BlockNumber block)
+{
+   Assert(block < scan->rs_nblocks);
+
+   /* release previous scan buffer, if any */
+   if (BufferIsValid(scan->rs_cbuf))
+   {
+       ReleaseBuffer(scan->rs_cbuf);
+       scan->rs_cbuf = InvalidBuffer;
+   }
+
+   /*
+    * Be sure to check for interrupts at least once per page.  Checks at
+    * higher code levels won't be able to stop a seqscan that encounters many
+    * pages' worth of consecutive dead tuples.
+    */
+   CHECK_FOR_INTERRUPTS();
+
+   /* read page using selected strategy */
+   scan->rs_cbuf = ReadBufferExtended(scan->rs_base.rs_rd, MAIN_FORKNUM, block,
+                                      RBM_NORMAL, scan->rs_strategy);
+   scan->rs_cblock = block;
+}
+
 /*
  * heapgettup_initial_block - return the first BlockNumber to scan
  *
@@ -748,7 +758,7 @@ heapgettup(HeapScanDesc scan,
     */
    while (block != InvalidBlockNumber)
    {
-       heapgetpage((TableScanDesc) scan, block);
+       heapfetchbuf(scan, block);
        LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
        page = heapgettup_start_page(scan, dir, &linesleft, &lineoff);
 continue_page:
@@ -869,7 +879,11 @@ heapgettup_pagemode(HeapScanDesc scan,
     */
    while (block != InvalidBlockNumber)
    {
-       heapgetpage((TableScanDesc) scan, block);
+       /* read the page */
+       heapfetchbuf(scan, block);
+
+       /* prune the page and determine visible tuple offsets */
+       heap_prepare_pagescan((TableScanDesc) scan);
        page = BufferGetPage(scan->rs_cbuf);
        linesleft = scan->rs_ntuples;
        lineindex = ScanDirectionIsForward(dir) ? 0 : linesleft - 1;

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 0952d4a98eb6844bf9b50e1183dc1cc51637c7b3..db99d2fcfa75649ae311df978155604995615ad2 100644 (file)
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2352,11 +2352,15 @@ heapam_scan_sample_next_block(TableScanDesc scan, SampleScanState *scanstate)
    if (hscan->rs_nblocks == 0)
        return false;
 
-   if (tsm->NextSampleBlock)
+   /* release previous scan buffer, if any */
+   if (BufferIsValid(hscan->rs_cbuf))
    {
-       blockno = tsm->NextSampleBlock(scanstate, hscan->rs_nblocks);
-       hscan->rs_cblock = blockno;
+       ReleaseBuffer(hscan->rs_cbuf);
+       hscan->rs_cbuf = InvalidBuffer;
    }
+
+   if (tsm->NextSampleBlock)
+       blockno = tsm->NextSampleBlock(scanstate, hscan->rs_nblocks);
    else
    {
        /* scanning table sequentially */
@@ -2398,20 +2402,32 @@ heapam_scan_sample_next_block(TableScanDesc scan, SampleScanState *scanstate)
        }
    }
 
+   hscan->rs_cblock = blockno;
+
    if (!BlockNumberIsValid(blockno))
    {
-       if (BufferIsValid(hscan->rs_cbuf))
-           ReleaseBuffer(hscan->rs_cbuf);
-       hscan->rs_cbuf = InvalidBuffer;
-       hscan->rs_cblock = InvalidBlockNumber;
        hscan->rs_inited = false;
-
        return false;
    }
 
-   heapgetpage(scan, blockno);
-   hscan->rs_inited = true;
+   Assert(hscan->rs_cblock < hscan->rs_nblocks);
+
+   /*
+    * Be sure to check for interrupts at least once per page.  Checks at
+    * higher code levels won't be able to stop a sample scan that encounters
+    * many pages' worth of consecutive dead tuples.
+    */
+   CHECK_FOR_INTERRUPTS();
+
+   /* Read page using selected strategy */
+   hscan->rs_cbuf = ReadBufferExtended(hscan->rs_base.rs_rd, MAIN_FORKNUM,
+                                       blockno, RBM_NORMAL, hscan->rs_strategy);
 
+   /* in pagemode, prune the page and determine visible tuple offsets */
+   if (hscan->rs_base.rs_flags & SO_ALLOW_PAGEMODE)
+       heap_prepare_pagescan(scan);
+
+   hscan->rs_inited = true;
    return true;
 }
 
@@ -2572,8 +2588,8 @@ SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
    if (scan->rs_flags & SO_ALLOW_PAGEMODE)
    {
        /*
-        * In pageatatime mode, heapgetpage() already did visibility checks,
-        * so just look at the info it left in rs_vistuples[].
+        * In pageatatime mode, heap_prepare_pagescan() already did visibility
+        * checks, so just look at the info it left in rs_vistuples[].
         *
         * We use a binary search over the known-sorted array.  Note: we could
         * save some effort if we insisted that NextSampleTuple select tuples

diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index a307fb5f2452fadf7307a6b5faf513058be19fe4..2765efc4e5e9cd995985afd79352313bcda1d46f 100644 (file)
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -267,7 +267,7 @@ extern TableScanDesc heap_beginscan(Relation relation, Snapshot snapshot,
                                    uint32 flags);
 extern void heap_setscanlimits(TableScanDesc sscan, BlockNumber startBlk,
                               BlockNumber numBlks);
-extern void heapgetpage(TableScanDesc sscan, BlockNumber block);
+extern void heap_prepare_pagescan(TableScanDesc sscan);
 extern void heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
                        bool allow_strat, bool allow_sync, bool allow_pagemode);
 extern void heap_endscan(TableScanDesc sscan);