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);
        }