Modify keys_are_unique optimization to release buffer pins before it

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
index 18eee722d3b8b33caf54b6ecea24ffb54abadc9f..a99b02c3c608ac77c054ed2d6260b218a52677d2 100644 (file)
--- 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
index 2f0c6aac529f585af8a649903f4e18e04b0aeb72..90e9af63c281c65d1b33e6be7128979533ef0519 100644 (file)
--- 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... */