Make heap_fetch API more consistent by having the buffer remain pinned

in all cases when keep_buf = true.  This allows ANALYZE's inner loop to
use heap_release_fetch, which saves multiple buffer lookups for the same
page and avoids overestimation of cost by the vacuum cost mechanism.

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index af5325ea632a82e198ba5c8253b88d089e72bf1f..c2d4395807fefcb2507d71b2c8f81f03bc1d728b 100644 (file)
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.179 2004/10/15 22:39:42 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.180 2004/10/26 16:05:02 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -878,14 +878,16 @@ heap_getnext(HeapScanDesc scan, ScanDirection direction)
  * is set to the buffer holding the tuple and TRUE is returned.  The caller
  * must unpin the buffer when done with the tuple.
  *
- * If the tuple is not found, then tuple->t_data is set to NULL, *userbuf
- * is set to InvalidBuffer, and FALSE is returned.
+ * If the tuple is not found (ie, item number references a deleted slot),
+ * then tuple->t_data is set to NULL and FALSE is returned.
  *
- * If the tuple is found but fails the time qual check, then FALSE will be
- * returned. When the caller specifies keep_buf = true, we retain the pin
- * on the buffer and return it in *userbuf (so the caller can still access
- * the tuple); when keep_buf = false, the pin is released and *userbuf is set
- * to InvalidBuffer.
+ * If the tuple is found but fails the time qual check, then FALSE is returned
+ * but tuple->t_data is left pointing to the tuple.
+ *
+ * keep_buf determines what is done with the buffer in the FALSE-result cases.
+ * When the caller specifies keep_buf = true, we retain the pin on the buffer
+ * and return it in *userbuf (so the caller must eventually unpin it); when
+ * keep_buf = false, the pin is released and *userbuf is set to InvalidBuffer.
  *
  * It is somewhat inconsistent that we ereport() on invalid block number but
  * return false on invalid item number.  This is historical.  The only
@@ -914,6 +916,8 @@ heap_fetch(Relation relation,
  * InvalidBuffer on entry, that buffer will be released before reading
  * the new page.  This saves a separate ReleaseBuffer step and hence
  * one entry into the bufmgr when looping through multiple fetches.
+ * Also, if *userbuf is the same buffer that holds the target tuple,
+ * we avoid bufmgr manipulation altogether.
  */
 bool
 heap_release_fetch(Relation relation,
@@ -962,8 +966,13 @@ heap_release_fetch(Relation relation,
    if (!ItemIdIsUsed(lp))
    {
        LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
-       ReleaseBuffer(buffer);
-       *userbuf = InvalidBuffer;
+       if (keep_buf)
+           *userbuf = buffer;
+       else
+       {
+           ReleaseBuffer(buffer);
+           *userbuf = InvalidBuffer;
+       }
        tuple->t_datamcxt = NULL;
        tuple->t_data = NULL;
        return false;
@@ -1007,17 +1016,13 @@ heap_release_fetch(Relation relation,
 
    /* Tuple failed time qual, but maybe caller wants to see it anyway. */
    if (keep_buf)
-   {
        *userbuf = buffer;
-
-       return false;
+   else
+   {
+       ReleaseBuffer(buffer);
+       *userbuf = InvalidBuffer;
    }
 
-   /* Okay to release pin on buffer. */
-   ReleaseBuffer(buffer);
-
-   *userbuf = InvalidBuffer;
-
    return false;
 }
 

diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 32615a62c27a3250805b7fb677f51c62ed3ddef1..9f22cb2040b0128156dcc78c1d9da8fcf7f80533 100644 (file)
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.117 2004/10/15 22:39:49 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.118 2004/10/26 16:05:02 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -269,8 +269,8 @@ _bt_check_unique(Relation rel, BTItem btitem, Relation heapRel,
                        SetBufferCommitInfoNeedsSave(buf);
                    }
                    LockBuffer(hbuffer, BUFFER_LOCK_UNLOCK);
-                   ReleaseBuffer(hbuffer);
                }
+               ReleaseBuffer(hbuffer);
            }
        }
 

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 486e86d677c15956d678480fd3e55057a7a9b127..872edc42988d6aa335daffd21b80527150da9352 100644 (file)
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.77 2004/09/30 23:21:19 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.78 2004/10/26 16:05:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -822,11 +822,12 @@ acquire_sample_rows(Relation onerel, HeapTuple *rows, int targrows,
        for (targoffset = FirstOffsetNumber; targoffset <= maxoffset; targoffset++)
        {
            HeapTupleData targtuple;
-           Buffer      tupbuffer;
 
            ItemPointerSet(&targtuple.t_self, targblock, targoffset);
-           if (heap_fetch(onerel, SnapshotNow, &targtuple, &tupbuffer,
-                          false, NULL))
+           /* We use heap_release_fetch to avoid useless bufmgr traffic */
+           if (heap_release_fetch(onerel, SnapshotNow,
+                                  &targtuple, &targbuffer,
+                                  true, NULL))
            {
                /*
                 * The first targrows live rows are simply copied into the
@@ -869,9 +870,6 @@ acquire_sample_rows(Relation onerel, HeapTuple *rows, int targrows,
                    rowstoskip -= 1;
                }
 
-               /* must release the extra pin acquired by heap_fetch */
-               ReleaseBuffer(tupbuffer);
-
                liverows += 1;
            }
            else
@@ -886,7 +884,7 @@ acquire_sample_rows(Relation onerel, HeapTuple *rows, int targrows,
            }
        }
 
-       /* Now release the initial pin on the page */
+       /* Now release the pin on the page */
        ReleaseBuffer(targbuffer);
    }