In _bt_check_unique() loop, don't bother applying _bt_isequal() to

killed items; just skip to the next item immediately.  Only check for
key equality when we reach a non-killed item or the end of the index
page.  This saves key comparisons when there are lots of killed items,
as for example in a heavily-updated table that's not been vacuumed lately.
Seems to be a win for pgbench anyway.

diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 02d842434c2d1aa78c5aaa034f32964d1f080678..c8dbbfb04152e1f1091f5cc5e8c3e453073dffd9 100644 (file)
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.104 2003/08/04 02:39:57 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtinsert.c,v 1.105 2003/09/02 22:10:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -189,27 +189,39 @@ _bt_check_unique(Relation rel, BTItem btitem, Relation heapRel,
                BlockNumber nblkno;
 
                /*
-                * make sure the offset points to an actual key before trying to
-                * compare it...
+                * make sure the offset points to an actual item before trying to
+                * examine it...
                 */
                if (offset <= maxoff)
                {
-                       /*
-                        * _bt_compare returns 0 for (1,NULL) and (1,NULL) - this's
-                        * how we handling NULLs - and so we must not use _bt_compare
-                        * in real comparison, but only for ordering/finding items on
-                        * pages. - vadim 03/24/97
-                        */
-                       if (!_bt_isequal(itupdesc, page, offset, natts, itup_scankey))
-                               break;                  /* we're past all the equal tuples */
-
                        curitemid = PageGetItemId(page, offset);
 
                        /*
-                        * We can skip the heap fetch if the item is marked killed.
+                        * We can skip items that are marked killed.
+                        *
+                        * Formerly, we applied _bt_isequal() before checking the kill
+                        * flag, so as to fall out of the item loop as soon as possible.
+                        * However, in the presence of heavy update activity an index
+                        * may contain many killed items with the same key; running
+                        * _bt_isequal() on each killed item gets expensive.  Furthermore
+                        * it is likely that the non-killed version of each key appears
+                        * first, so that we didn't actually get to exit any sooner anyway.
+                        * So now we just advance over killed items as quickly as we can.
+                        * We only apply _bt_isequal() when we get to a non-killed item or
+                        * the end of the page.
                         */
                        if (!ItemIdDeleted(curitemid))
                        {
+                               /*
+                                * _bt_compare returns 0 for (1,NULL) and (1,NULL) - this's
+                                * how we handling NULLs - and so we must not use _bt_compare
+                                * in real comparison, but only for ordering/finding items on
+                                * pages. - vadim 03/24/97
+                                */
+                               if (!_bt_isequal(itupdesc, page, offset, natts, itup_scankey))
+                                       break;                  /* we're past all the equal tuples */
+
+                               /* okay, we gotta fetch the heap tuple ... */
                                cbti = (BTItem) PageGetItem(page, curitemid);
                                htup.t_self = cbti->bti_itup.t_tid;
                                if (heap_fetch(heapRel, SnapshotDirty, &htup, &hbuffer,
@@ -1547,7 +1559,7 @@ _bt_pgaddtup(Relation rel,
  * _bt_isequal - used in _bt_doinsert in check for duplicates.
  *
  * This is very similar to _bt_compare, except for NULL handling.
- * Rule is simple: NOT_NULL not equal NULL, NULL not_equal NULL too.
+ * Rule is simple: NOT_NULL not equal NULL, NULL not equal NULL too.
  */
 static bool
 _bt_isequal(TupleDesc itupdesc, Page page, OffsetNumber offnum,
@@ -1576,7 +1588,7 @@ _bt_isequal(TupleDesc itupdesc, Page page, OffsetNumber offnum,
                datum = index_getattr(itup, attno, itupdesc, &isNull);
 
                /* NULLs are never equal to anything */
-               if (entry->sk_flags & SK_ISNULL || isNull)
+               if ((entry->sk_flags & SK_ISNULL) || isNull)
                        return false;
 
                result = DatumGetInt32(FunctionCall2(&entry->sk_func,