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.

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

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

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