1. _bt_compare fixed to work properly with new code in _bt_insertonpg

   (old _bt_compare always returned >= 0 while comparing with P_HIKEY
   on root page - it breaks root page when _bt_insertonpg tries insert
   new minimal key into root page).
2. Fixed bug concerns "empty" pages: non-rightmost pages with only P_HIKEY
   present on it. Such pages appear after vacuum.

diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
index b3c5e007cad68baa12a0a3f4a7f4a8d6f0f74820..61a26c0f7d19f6de0af8a7aacfa30113cfbcad43 100644 (file)
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *    $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.10 1996/11/21 06:10:55 vadim Exp $
+ *    $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.11 1996/12/06 09:41:45 vadim Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -409,9 +409,19 @@ _bt_firsteq(Relation rel,
  *      0 if scankey == tuple at offnum;
  *     +1 if scankey > tuple at offnum.
  *
+ * -- Old comments:
  * In order to avoid having to propagate changes up the tree any time
  * a new minimal key is inserted, the leftmost entry on the leftmost
  * page is less than all possible keys, by definition.
+ *
+ * -- New ones:
+ * New insertion code (fix against updating _in_place_ if new minimal
+ * key has bigger size than old one) may delete P_HIKEY entry on the
+ * root page in order to insert new minimal key - and so this definition 
+ * does not work properly in this case and breaks key' order on root 
+ * page. BTW, this propagation occures only while page' splitting, 
+ * but not "any time a new min key is inserted" (see _bt_insertonpg). 
+ *     - vadim 12/05/96
  */
 int
 _bt_compare(Relation rel,
@@ -436,6 +446,8 @@ _bt_compare(Relation rel,
      *  If this is a leftmost internal page, and if our comparison is
      *  with the first key on the page, then the item at that position is
      *  by definition less than the scan key.
+     *
+     *  - see new comments above...
      */
     
     opaque = (BTPageOpaque) PageGetSpecialPointer(page);
@@ -453,12 +465,20 @@ _bt_compare(Relation rel,
     *  well as the rightmost page.  but that implies that this
     *  code path only applies to the root -- which seems
     *  unlikely..
+    *
+    *  - see new comments above...
     */
    if (! P_RIGHTMOST(opaque)) {
        elog(WARN, "_bt_compare: invalid comparison to high key");
    }
 
+#ifdef 0
    /*
+    *  We just have to belive that right answer will not
+    *  break anything. I've checked code and all seems to be ok.
+    *  See new comments above...
+    *
+    *  -- Old comments
     *  If the item on the page is equal to the scankey, that's
     *  okay to admit.  We just can't claim that the first key on
     *  the page is greater than anything.
@@ -469,6 +489,7 @@ _bt_compare(Relation rel,
        return (0);
    }
    return (1);
+#endif
     }
     
     btitem = (BTItem) PageGetItem(page, PageGetItemId(page, offnum));
@@ -601,6 +622,7 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
     Page page;
     BTStack stack;
     OffsetNumber offnum, maxoff;
+    bool offGmax = false;
     BTItem btitem;
     IndexTuple itup;
     ItemPointer current;
@@ -665,7 +687,10 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
     maxoff = PageGetMaxOffsetNumber(page);
     
     if (offnum > maxoff)
+    {
    offnum = maxoff;
+   offGmax = true;
+    }
     
     blkno = BufferGetBlockNumber(buf);
     ItemPointerSet(current, blkno, offnum);
@@ -736,10 +761,21 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
        if (result > 0)
        (void) _bt_twostep(scan, &buf, ForwardScanDirection);
    }
+   else if ( offGmax && result > 0 )
+   {   /*
+        * Just remember:  _bt_binsrch() returns the OffsetNumber of 
+        * the first matching key on the page, or the OffsetNumber at 
+        * which the matching key WOULD APPEAR IF IT WERE on this page.
+        * No key on this page, but offnum from _bt_binsrch() greater
+        * maxoff - have to move right. - vadim 12/06/96
+        */
+       (void) _bt_twostep(scan, &buf, ForwardScanDirection);
+   }
    break;
    
     case BTGreaterStrategyNumber:
-   if (result >= 0) {
+   /* offGmax helps as above */
+   if (result >= 0 || offGmax) {
        do {
        if (!_bt_twostep(scan, &buf, ForwardScanDirection))
            break;
@@ -1084,13 +1120,17 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir)
     maxoff = PageGetMaxOffsetNumber(page);
     
     if (ScanDirectionIsForward(dir)) {
-   if (PageIsEmpty(page)) {
-       maxoff = FirstOffsetNumber;
-   } else {
-       maxoff = PageGetMaxOffsetNumber(page);
-   }
+   if ( !P_LEFTMOST(opaque) )  /* non-leftmost page ? */
+       elog (WARN, "_bt_endpoint: leftmost page (%u) has not leftmost flag", blkno);
    start = P_RIGHTMOST(opaque) ? P_HIKEY : P_FIRSTKEY;
-   
+   /*
+    * I don't understand this stuff! It doesn't work for non-rightmost
+    * pages with only one element (P_HIKEY) which we have after
+    * deletion itups by vacuum (it's case of start > maxoff).
+    * Scanning in BackwardScanDirection is not understandable at all.
+    * Well - new stuff. - vadim 12/06/96
+    */
+#ifdef 0
    if (PageIsEmpty(page) || start > maxoff) {
        ItemPointerSet(current, blkno, maxoff);
        if (!_bt_step(scan, &buf, BackwardScanDirection))
@@ -1098,10 +1138,35 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir)
        
        start = ItemPointerGetOffsetNumber(current);
        page = BufferGetPage(buf);
-   } else {
+   }
+#endif
+   if ( PageIsEmpty (page) )
+   {
+       if ( start != P_HIKEY ) /* non-rightmost page */
+       elog (WARN, "_bt_endpoint: non-rightmost page (%u) is empty", blkno);
+       /* It's left- & right- most page - root page, - and it's empty... */
+       return ((RetrieveIndexResult) NULL);
+   }
+   if ( start > maxoff )       /* start == 2 && maxoff == 1 */
+   {
+       ItemPointerSet(current, blkno, maxoff);
+       if (!_bt_step(scan, &buf, ForwardScanDirection))
+       return ((RetrieveIndexResult) NULL);
+       
+       start = ItemPointerGetOffsetNumber(current);
+       page = BufferGetPage(buf);
+   }
+   /* new stuff ends here */
+   else {
        ItemPointerSet(current, blkno, start);
    }
     } else if (ScanDirectionIsBackward(dir)) {
+   /*
+    * I don't understand this stuff too! If RIGHT-most leaf page is
+    * empty why do scanning in ForwardScanDirection ???
+    * Well - new stuff. - vadim 12/06/96
+    */
+#ifdef 0
    if (PageIsEmpty(page)) {
        ItemPointerSet(current, blkno, FirstOffsetNumber);
        if (!_bt_step(scan, &buf, ForwardScanDirection))
@@ -1109,7 +1174,23 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir)
        
        start = ItemPointerGetOffsetNumber(current);
        page = BufferGetPage(buf);
-   } else {
+   }
+#endif
+   if (PageIsEmpty(page))
+   {
+       /* If it's leftmost page too - it's empty root page... */
+       if ( P_LEFTMOST(opaque) )
+           return ((RetrieveIndexResult) NULL);
+       /* Go back ! */
+       ItemPointerSet(current, blkno, FirstOffsetNumber);
+       if (!_bt_step(scan, &buf, BackwardScanDirection))
+       return ((RetrieveIndexResult) NULL);
+       
+       start = ItemPointerGetOffsetNumber(current);
+       page = BufferGetPage(buf);
+   }
+   /* new stuff ends here */
+   else {
        start = PageGetMaxOffsetNumber(page);
        ItemPointerSet(current, blkno, start);
    }