Modify keys_are_unique optimization to release buffer pins before it

tglsfdc · tglsfdc · commit fddc2d94cea0 · 2003-03-24T21:42:33.000Z
returns NULL.  This avoids out-of-buffers failures during many-way
indexscans, as in Shraibman's complaint of 21-Mar.
diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.37 2003/01/08 19:41:40 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.38 2003/03/24 21:42:33 tgl Exp $
  *
  * NOTES
  *	  many of the old access method routines have been turned into
@@ -91,7 +91,7 @@ RelationGetIndexScan(Relation indexRelation,
 
 	scan->kill_prior_tuple = false;
 	scan->ignore_killed_tuples = true;	/* default setting */
-	scan->keys_are_unique = false;		/* may be set by amrescan */
+	scan->keys_are_unique = false;		/* may be set by index AM */
 	scan->got_tuple = false;
 
 	scan->opaque = NULL;
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/index/indexam.c,v 1.65 2003/03/23 23:01:03 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/index/indexam.c,v 1.66 2003/03/24 21:42:33 tgl Exp $
  *
  * INTERFACE ROUTINES
  *		index_open		- open an index relation by relation OID
@@ -311,7 +311,7 @@ index_rescan(IndexScanDesc scan, ScanKey key)
 	GET_SCAN_PROCEDURE(rescan, amrescan);
 
 	scan->kill_prior_tuple = false;		/* for safety */
-	scan->keys_are_unique = false;		/* may be set by amrescan */
+	scan->keys_are_unique = false;		/* may be set by index AM */
 	scan->got_tuple = false;
 	scan->unique_tuple_pos = 0;
 	scan->unique_tuple_mark = 0;
@@ -413,37 +413,70 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
 
 	SCAN_CHECKS;
 
+	/* Release any previously held pin */
+	if (BufferIsValid(scan->xs_cbuf))
+	{
+		ReleaseBuffer(scan->xs_cbuf);
+		scan->xs_cbuf = InvalidBuffer;
+	}
+
 	/*
-	 * Can skip entering the index AM if we already got a tuple and it
-	 * must be unique.  Instead, we need a "short circuit" path that
-	 * just keeps track of logical scan position (before/on/after tuple).
+	 * If we already got a tuple and it must be unique, there's no need
+	 * to make the index AM look through any additional tuples.  (This can
+	 * save a useful amount of work in scenarios where there are many dead
+	 * tuples due to heavy update activity.)
 	 *
-	 * Note that we hold the pin on the single tuple's buffer throughout
-	 * the scan once we are in this state.
+	 * To do this we must keep track of the logical scan position
+	 * (before/on/after tuple).  Also, we have to be sure to release scan
+	 * resources before returning NULL; if we fail to do so then a multi-index
+	 * scan can easily run the system out of free buffers.  We can release
+	 * index-level resources fairly cheaply by calling index_rescan.  This
+	 * means there are two persistent states as far as the index AM is
+	 * concerned: on-tuple and rescanned.  If we are actually asked to
+	 * re-fetch the single tuple, we have to go through a fresh indexscan
+	 * startup, which penalizes that (infrequent) case.
 	 */
 	if (scan->keys_are_unique && scan->got_tuple)
 	{
+		int		new_tuple_pos = scan->unique_tuple_pos;
+
 		if (ScanDirectionIsForward(direction))
 		{
-			if (scan->unique_tuple_pos <= 0)
-				scan->unique_tuple_pos++;
+			if (new_tuple_pos <= 0)
+				new_tuple_pos++;
+		}
+		else
+		{
+			if (new_tuple_pos >= 0)
+				new_tuple_pos--;
 		}
-		else if (ScanDirectionIsBackward(direction))
+		if (new_tuple_pos == 0)
 		{
-			if (scan->unique_tuple_pos >= 0)
-				scan->unique_tuple_pos--;
+			/*
+			 * We are moving onto the unique tuple from having been off it.
+			 * We just fall through and let the index AM do the work.  Note
+			 * we should get the right answer regardless of scan direction.
+			 */
+			scan->unique_tuple_pos = 0;	/* need to update position */
 		}
-		if (scan->unique_tuple_pos == 0)
-			return heapTuple;
 		else
-			return NULL;
-	}
+		{
+			/*
+			 * Moving off the tuple; must do amrescan to release index-level
+			 * pins before we return NULL.  Since index_rescan will reset
+			 * my state, must save and restore...
+			 */
+			int		unique_tuple_mark = scan->unique_tuple_mark;
 
-	/* Release any previously held pin */
-	if (BufferIsValid(scan->xs_cbuf))
-	{
-		ReleaseBuffer(scan->xs_cbuf);
-		scan->xs_cbuf = InvalidBuffer;
+			index_rescan(scan, NULL /* no change to key */);
+
+			scan->keys_are_unique = true;
+			scan->got_tuple = true;
+			scan->unique_tuple_pos = new_tuple_pos;
+			scan->unique_tuple_mark = unique_tuple_mark;
+
+			return NULL;
+		}
 	}
 
 	/* just make sure this is false... */

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`	`*`
`9`	`9`	`*`
`10`	`10`	`* IDENTIFICATION`
`11`		`- * $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.37 2003/01/08 19:41:40 tgl Exp $`
	`11`	`+ * $Header: /cvsroot/pgsql/src/backend/access/index/genam.c,v 1.38 2003/03/24 21:42:33 tgl Exp $`
`12`	`12`	`*`
`13`	`13`	`* NOTES`
`14`	`14`	`* many of the old access method routines have been turned into`
`@@ -91,7 +91,7 @@ RelationGetIndexScan(Relation indexRelation,`
`91`	`91`
`92`	`92`	`scan->kill_prior_tuple = false;`
`93`	`93`	`scan->ignore_killed_tuples = true; /* default setting */`
`94`		`- scan->keys_are_unique = false; /* may be set by amrescan */`
	`94`	`+ scan->keys_are_unique = false; /* may be set by index AM */`
`95`	`95`	`scan->got_tuple = false;`
`96`	`96`
`97`	`97`	`scan->opaque = NULL;`