Remove RelationGetBufferWithBuffer(), which is horribly confused about

appropriate pin-count manipulation, and instead use ReleaseAndReadBuffer.
Make use of the fact that the passed-in buffer (if there is one) must
be pinned to avoid grabbing the bufmgr spinlock when we are able to
return this same buffer.  Eliminate unnecessary 'previous tuple' and
'next tuple' fields of HeapScanDesc and IndexScanDesc, thereby removing
a whole lot of bookkeeping from heap_getnext() and related routines.

src/backend/access/gist/gistscan.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/gist/gistscan.c,v 1.35 2001/05/31 18:16:54 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/gist/gistscan.c,v 1.36 2001/06/09 18:16:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -83,13 +83,8 @@ gistrescan(PG_FUNCTION_ARGS)
 	/*
 	 * Clear all the pointers.
 	 */
-
-	ItemPointerSetInvalid(&s->previousItemData);
 	ItemPointerSetInvalid(&s->currentItemData);
-	ItemPointerSetInvalid(&s->nextItemData);
-	ItemPointerSetInvalid(&s->previousMarkData);
 	ItemPointerSetInvalid(&s->currentMarkData);
-	ItemPointerSetInvalid(&s->nextMarkData);
 
 	/*
 	 * Set flags.

src/backend/access/index/genam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.26 2001/01/24 19:42:48 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.27 2001/06/09 18:16:56 tgl Exp $
  *
  * NOTES
  *	  many of the old access method routines have been turned into
@@ -107,12 +107,8 @@ RelationGetIndexScan(Relation relation,
 	scan->opaque = NULL;
 	scan->numberOfKeys = numberOfKeys;
 
-	ItemPointerSetInvalid(&scan->previousItemData);
 	ItemPointerSetInvalid(&scan->currentItemData);
-	ItemPointerSetInvalid(&scan->nextItemData);
-	ItemPointerSetInvalid(&scan->previousMarkData);
 	ItemPointerSetInvalid(&scan->currentMarkData);
-	ItemPointerSetInvalid(&scan->nextMarkData);
 
 	/*
 	 * mark cached function lookup data invalid; it will be set on first
@@ -176,9 +172,7 @@ IndexScanRestart(IndexScanDesc scan,
 	if (!IndexScanIsValid(scan))
 		elog(ERROR, "IndexScanRestart: invalid scan");
 
-	ItemPointerSetInvalid(&scan->previousItemData);
 	ItemPointerSetInvalid(&scan->currentItemData);
-	ItemPointerSetInvalid(&scan->nextItemData);
 
 	if (RelationGetNumberOfBlocks(scan->relation) == 0)
 		scan->flags = ScanUnmarked;
@@ -213,37 +207,7 @@ IndexScanRestart(IndexScanDesc scan,
 void
 IndexScanMarkPosition(IndexScanDesc scan)
 {
-	RetrieveIndexResult result;
-
-	if (scan->flags & ScanUncheckedPrevious)
-	{
-		result = index_getnext(scan, BackwardScanDirection);
-
-		if (result != NULL)
-		{
-			scan->previousItemData = result->index_iptr;
-			pfree(result);
-		}
-		else
-			ItemPointerSetInvalid(&scan->previousItemData);
-	}
-	else if (scan->flags & ScanUncheckedNext)
-	{
-		result = (RetrieveIndexResult)
-			index_getnext(scan, ForwardScanDirection);
-
-		if (result != NULL)
-		{
-			scan->nextItemData = result->index_iptr;
-			pfree(result);
-		}
-		else
-			ItemPointerSetInvalid(&scan->nextItemData);
-	}
-
-	scan->previousMarkData = scan->previousItemData;
 	scan->currentMarkData = scan->currentItemData;
-	scan->nextMarkData = scan->nextItemData;
 
 	scan->flags = 0x0;			/* XXX should have a symbolic name */
 }
@@ -269,9 +233,7 @@ IndexScanRestorePosition(IndexScanDesc scan)
 	if (scan->flags & ScanUnmarked)
 		elog(ERROR, "IndexScanRestorePosition: no mark to restore");
 
-	scan->previousItemData = scan->previousMarkData;
 	scan->currentItemData = scan->currentMarkData;
-	scan->nextItemData = scan->nextMarkData;
 
 	scan->flags = 0x0;			/* XXX should have a symbolic name */
 }

src/backend/access/rtree/rtscan.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/rtree/Attic/rtscan.c,v 1.36 2001/03/22 03:59:16 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/rtree/Attic/rtscan.c,v 1.37 2001/06/09 18:16:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -85,13 +85,8 @@ rtrescan(PG_FUNCTION_ARGS)
 	/*
 	 * Clear all the pointers.
 	 */
-
-	ItemPointerSetInvalid(&s->previousItemData);
 	ItemPointerSetInvalid(&s->currentItemData);
-	ItemPointerSetInvalid(&s->nextItemData);
-	ItemPointerSetInvalid(&s->previousMarkData);
 	ItemPointerSetInvalid(&s->currentMarkData);
-	ItemPointerSetInvalid(&s->nextMarkData);
 
 	/*
 	 * Set flags.

src/backend/storage/buffer/bufmgr.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.111 2001/05/12 19:58:27 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/storage/buffer/bufmgr.c,v 1.112 2001/06/09 18:16:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -96,43 +96,6 @@ static int	ReleaseBufferWithBufferLock(Buffer buffer);
 static int	BufferReplace(BufferDesc *bufHdr);
 void		PrintBufferDescs(void);
 
-/* ---------------------------------------------------
- * RelationGetBufferWithBuffer
- *		see if the given buffer is what we want
- *		if yes, we don't need to bother the buffer manager
- * ---------------------------------------------------
- */
-Buffer
-RelationGetBufferWithBuffer(Relation relation,
-							BlockNumber blockNumber,
-							Buffer buffer)
-{
-	BufferDesc *bufHdr;
-
-	if (BufferIsValid(buffer))
-	{
-		if (!BufferIsLocal(buffer))
-		{
-			bufHdr = &BufferDescriptors[buffer - 1];
-			SpinAcquire(BufMgrLock);
-			if (bufHdr->tag.blockNum == blockNumber &&
-				RelFileNodeEquals(bufHdr->tag.rnode, relation->rd_node))
-			{
-				SpinRelease(BufMgrLock);
-				return buffer;
-			}
-			return ReadBufferInternal(relation, blockNumber, false, true);
-		}
-		else
-		{
-			bufHdr = &LocalBufferDescriptors[-buffer - 1];
-			if (bufHdr->tag.blockNum == blockNumber &&
-				RelFileNodeEquals(bufHdr->tag.rnode, relation->rd_node))
-				return buffer;
-		}
-	}
-	return ReadBufferInternal(relation, blockNumber, false, false);
-}
 
 /*
  * ReadBuffer -- returns a buffer containing the requested
@@ -141,7 +104,8 @@ RelationGetBufferWithBuffer(Relation relation,
  *		allocate a new block.
  *
  * Returns: the buffer number for the buffer containing
- *		the block read or NULL on an error.
+ *		the block read, or NULL on an error.  If successful,
+ *		the returned buffer has been pinned.
  *
  * Assume when this function is called, that reln has been
  *		opened already.
@@ -300,8 +264,8 @@ ReadBufferInternal(Relation reln, BlockNumber blockNum,
 }
 
 /*
- * BufferAlloc -- Get a buffer from the buffer pool but dont
- *		read it.
+ * BufferAlloc -- Get a buffer from the buffer pool but don't
+ *		read it.  If successful, the returned buffer is pinned.
  *
  * Returns: descriptor for buffer
  *
@@ -684,6 +648,11 @@ WriteNoReleaseBuffer(Buffer buffer)
  * Note: it is OK to pass buffer = InvalidBuffer, indicating that no old
  * buffer actually needs to be released.  This case is the same as ReadBuffer
  * except for the isExtend option.
+ *
+ * Also, if the passed buffer is valid and already contains the desired block
+ * number, we simply return it without ever acquiring the spinlock at all.
+ * Since the passed buffer must be pinned, it's OK to examine its block
+ * number without getting the lock first.
  */
 Buffer
 ReleaseAndReadBuffer(Buffer buffer,
@@ -698,12 +667,19 @@ ReleaseAndReadBuffer(Buffer buffer,
 		if (BufferIsLocal(buffer))
 		{
 			Assert(LocalRefCount[-buffer - 1] > 0);
+			bufHdr = &LocalBufferDescriptors[-buffer - 1];
+			if (bufHdr->tag.blockNum == blockNum &&
+				RelFileNodeEquals(bufHdr->tag.rnode, relation->rd_node))
+				return buffer;
 			LocalRefCount[-buffer - 1]--;
 		}
 		else
 		{
-			bufHdr = &BufferDescriptors[buffer - 1];
 			Assert(PrivateRefCount[buffer - 1] > 0);
+			bufHdr = &BufferDescriptors[buffer - 1];
+			if (bufHdr->tag.blockNum == blockNum &&
+				RelFileNodeEquals(bufHdr->tag.rnode, relation->rd_node))
+				return buffer;
 			if (PrivateRefCount[buffer - 1] > 1)
 				PrivateRefCount[buffer - 1]--;
 			else
@@ -1002,7 +978,7 @@ BufferPoolCheckLeak()
 			BufferDesc *buf = &(BufferDescriptors[i - 1]);
 
 			elog(NOTICE,
-				 "Buffer Leak: [%03d] (freeNext=%ld, freePrev=%ld, \
+				 "Buffer Leak: [%03d] (freeNext=%d, freePrev=%d, \
 relname=%s, blockNum=%d, flags=0x%x, refcount=%d %ld)",
 				 i - 1, buf->freeNext, buf->freePrev,
 				 buf->blind.relname, buf->tag.blockNum, buf->flags,
@@ -1396,7 +1372,7 @@ PrintBufferDescs()
 		SpinAcquire(BufMgrLock);
 		for (i = 0; i < NBuffers; ++i, ++buf)
 		{
-			elog(DEBUG, "[%02d] (freeNext=%ld, freePrev=%ld, relname=%s, \
+			elog(DEBUG, "[%02d] (freeNext=%d, freePrev=%d, relname=%s, \
 blockNum=%d, flags=0x%x, refcount=%d %ld)",
 				 i, buf->freeNext, buf->freePrev,
 				 buf->blind.relname, buf->tag.blockNum, buf->flags,
@@ -1426,7 +1402,7 @@ PrintPinnedBufs()
 	for (i = 0; i < NBuffers; ++i, ++buf)
 	{
 		if (PrivateRefCount[i] > 0)
-			elog(NOTICE, "[%02d] (freeNext=%ld, freePrev=%ld, relname=%s, \
+			elog(NOTICE, "[%02d] (freeNext=%d, freePrev=%d, relname=%s, \
 blockNum=%d, flags=0x%x, refcount=%d %ld)\n",
 				 i, buf->freeNext, buf->freePrev, buf->blind.relname,
 				 buf->tag.blockNum, buf->flags,
@@ -1719,7 +1695,7 @@ IncrBufferRefCount_Debug(char *file, int line, Buffer buffer)
 	{
 		BufferDesc *buf = &BufferDescriptors[buffer - 1];
 
-		fprintf(stderr, "PIN(Incr) %ld relname = %s, blockNum = %d, \
+		fprintf(stderr, "PIN(Incr) %d relname = %s, blockNum = %d, \
 refcount = %ld, file: %s, line: %d\n",
 				buffer, buf->blind.relname, buf->tag.blockNum,
 				PrivateRefCount[buffer - 1], file, line);
@@ -1737,7 +1713,7 @@ ReleaseBuffer_Debug(char *file, int line, Buffer buffer)
 	{
 		BufferDesc *buf = &BufferDescriptors[buffer - 1];
 
-		fprintf(stderr, "UNPIN(Rel) %ld relname = %s, blockNum = %d, \
+		fprintf(stderr, "UNPIN(Rel) %d relname = %s, blockNum = %d, \
 refcount = %ld, file: %s, line: %d\n",
 				buffer, buf->blind.relname, buf->tag.blockNum,
 				PrivateRefCount[buffer - 1], file, line);
@@ -1765,7 +1741,7 @@ ReleaseAndReadBuffer_Debug(char *file,
 	{
 		BufferDesc *buf = &BufferDescriptors[buffer - 1];
 
-		fprintf(stderr, "UNPIN(Rel&Rd) %ld relname = %s, blockNum = %d, \
+		fprintf(stderr, "UNPIN(Rel&Rd) %d relname = %s, blockNum = %d, \
 refcount = %ld, file: %s, line: %d\n",
 				buffer, buf->blind.relname, buf->tag.blockNum,
 				PrivateRefCount[buffer - 1], file, line);
@@ -1774,7 +1750,7 @@ refcount = %ld, file: %s, line: %d\n",
 	{
 		BufferDesc *buf = &BufferDescriptors[b - 1];
 
-		fprintf(stderr, "PIN(Rel&Rd) %ld relname = %s, blockNum = %d, \
+		fprintf(stderr, "PIN(Rel&Rd) %d relname = %s, blockNum = %d, \
 refcount = %ld, file: %s, line: %d\n",
 				b, buf->blind.relname, buf->tag.blockNum,
 				PrivateRefCount[b - 1], file, line);
@@ -2057,7 +2033,7 @@ LockBuffer(Buffer buffer, int mode)
 		{
 			S_UNLOCK(&(buf->cntx_lock));
 			RESUME_INTERRUPTS();
-			elog(ERROR, "UNLockBuffer: buffer %lu is not locked", buffer);
+			elog(ERROR, "UNLockBuffer: buffer %d is not locked", buffer);
 		}
 	}
 	else if (mode == BUFFER_LOCK_SHARE)

src/include/access/relscan.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: relscan.h,v 1.20 2001/01/24 19:43:19 momjian Exp $
+ * $Id: relscan.h,v 1.21 2001/06/09 18:16:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -16,25 +16,16 @@
 
 #include "utils/tqual.h"
 
-typedef ItemPointerData MarkData;
 
 typedef struct HeapScanDescData
 {
 	Relation	rs_rd;			/* pointer to relation descriptor */
-	HeapTupleData rs_ptup;		/* previous tuple in scan */
-	HeapTupleData rs_ctup;		/* current tuple in scan */
-	HeapTupleData rs_ntup;		/* next tuple in scan */
-	Buffer		rs_pbuf;		/* previous buffer in scan */
-	Buffer		rs_cbuf;		/* current buffer in scan */
-	Buffer		rs_nbuf;		/* next buffer in scan */
-	ItemPointerData rs_mptid;	/* marked previous tid */
-	ItemPointerData rs_mctid;	/* marked current tid */
-	ItemPointerData rs_mntid;	/* marked next tid */
-	ItemPointerData rs_mcd;		/* marked current delta XXX ??? */
+	HeapTupleData rs_ctup;		/* current tuple 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 */
+	ItemPointerData rs_mctid;	/* marked tid, if any */
 	Snapshot	rs_snapshot;	/* snapshot to see */
-	bool		rs_atend;		/* restart scan at end? */
-	uint16		rs_cdelta;		/* current delta in chain */
-	uint16		rs_nkeys;		/* number of attributes in keys */
+	uint16		rs_nkeys;		/* number of scan keys to select tuples */
 	ScanKey		rs_key;			/* key descriptors */
 } HeapScanDescData;
 
@@ -43,22 +34,24 @@ typedef HeapScanDescData *HeapScanDesc;
 typedef struct IndexScanDescData
 {
 	Relation	relation;		/* relation descriptor */
-	void	   *opaque;			/* am-specific slot */
-	ItemPointerData previousItemData;	/* previous index pointer */
+	void	   *opaque;			/* access-method-specific info */
 	ItemPointerData currentItemData;	/* current index pointer */
-	ItemPointerData nextItemData;		/* next index pointer */
-	MarkData	previousMarkData;		/* marked previous pointer */
-	MarkData	currentMarkData;/* marked current  pointer */
-	MarkData	nextMarkData;	/* marked next pointer */
+	ItemPointerData	currentMarkData;	/* marked current pointer */
 	uint8		flags;			/* scan position flags */
 	bool		scanFromEnd;	/* restart scan at end? */
-	uint16		numberOfKeys;	/* number of key attributes */
-	ScanKey		keyData;		/* key descriptor */
+	uint16		numberOfKeys;	/* number of scan keys to select tuples */
+	ScanKey		keyData;		/* key descriptors */
 	FmgrInfo	fn_getnext;		/* cached lookup info for am's getnext fn */
 } IndexScanDescData;
 
 typedef IndexScanDescData *IndexScanDesc;
 
+/* IndexScanDesc flag bits (none of these are actually used currently) */
+#define ScanUnmarked			0x01
+#define ScanUncheckedPrevious	0x02
+#define ScanUncheckedNext		0x04
+
+
 /* ----------------
  *		IndexScanDescPtr is used in the executor where we have to
  *		keep track of several index scans when using several indices

src/include/access/skey.h

@@ -7,8 +7,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: skey.h,v 1.15 2001/06/01 02:41:36 tgl Exp $
- *
+ * $Id: skey.h,v 1.16 2001/06/09 18:16:59 tgl Exp $
  *
  * Note:
  *		Needs more accessor/assignment routines.
@@ -20,6 +19,7 @@
 #include "access/attnum.h"
 #include "fmgr.h"
 
+
 typedef struct ScanKeyData
 {
 	bits16		sk_flags;		/* flags */
@@ -38,11 +38,6 @@ typedef ScanKeyData *ScanKey;
 #define SK_COMMUTE		0x8		/* commute function (not fully supported) */
 
 
-#define ScanUnmarked			0x01
-#define ScanUncheckedPrevious	0x02
-#define ScanUncheckedNext		0x04
-
-
 /*
  * prototypes for functions in access/common/scankey.c
  */

src/include/storage/buf.h

@@ -7,17 +7,22 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: buf.h,v 1.8 2001/01/24 19:43:27 momjian Exp $
+ * $Id: buf.h,v 1.9 2001/06/09 18:16:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #ifndef BUF_H
 #define BUF_H
 
-#define InvalidBuffer	(0)
-#define UnknownBuffer	(-99999)
+/*
+ * Buffer identifiers.
+ *
+ * Zero is invalid, positive is the index of a shared buffer (1..NBuffers),
+ * negative is the index of a local buffer (-1 .. -NLocBuffer).
+ */
+typedef int Buffer;
 
-typedef long Buffer;
+#define InvalidBuffer	0
 
 /*
  * BufferIsInvalid
@@ -25,12 +30,6 @@ typedef long Buffer;
  */
 #define BufferIsInvalid(buffer) ((buffer) == InvalidBuffer)
 
-/*
- * BufferIsUnknown
- *		True iff the buffer is unknown.
- */
-#define BufferIsUnknown(buffer) ((buffer) == UnknownBuffer)
-
 /*
  * BufferIsLocal
  *		True iff the buffer is local (not visible to other servers).

src/include/storage/bufmgr.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: bufmgr.h,v 1.51 2001/05/12 19:58:28 tgl Exp $
+ * $Id: bufmgr.h,v 1.52 2001/06/09 18:16:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -75,10 +75,6 @@ extern long *LocalRefCount;
  *		True iff the given buffer number is valid (either as a shared
  *		or local buffer).
  *
- * Note:
- *		BufferIsValid(InvalidBuffer) is False.
- *		BufferIsValid(UnknownBuffer) is False.
- *
  * Note: For a long time this was defined the same as BufferIsPinned,
  * that is it would say False if you didn't hold a pin on the buffer.
  * I believe this was bogus and served only to mask logic errors.
@@ -158,8 +154,6 @@ extern long *LocalRefCount;
 /*
  * prototypes for functions in bufmgr.c
  */
-extern Buffer RelationGetBufferWithBuffer(Relation relation,
-							BlockNumber blockNumber, Buffer buffer);
 extern Buffer ReadBuffer(Relation reln, BlockNumber blockNum);
 extern int	WriteBuffer(Buffer buffer);
 extern int	WriteNoReleaseBuffer(Buffer buffer);