Merge spetrunia@bk-internal.mysql.com:/home/bk/mysql-4.1

into mysql.com:/dbdata/psergey/mysql-4.1-heap-bug


heap/hp_hash.c:
  Auto merged
mysql-test/r/heap.result:
  Auto merged
sql/ha_heap.cc:
  Auto merged

7 files changed, 180 insertions, 26 deletions

diff --git a/heap/_check.c b/heap/_check.c
index 233cb8cb0c5..a745aee48bf 100644
--- a/heap/_check.c
+++ b/heap/_check.c
@@ -102,9 +102,11 @@ static int check_one_key(HP_KEYDEF *keydef, uint keynr, ulong records,
   int error;
   uint i,found,max_links,seek,links;
   uint rec_link;				/* Only used with debugging */
+  uint hash_buckets_found;
   HASH_INFO *hash_info;
 
   error=0;
+  hash_buckets_found= 0;
   for (i=found=max_links=seek=0 ; i < records ; i++)
   {
     hash_info=hp_find_hash(&keydef->block,i);
@@ -128,21 +130,32 @@ static int check_one_key(HP_KEYDEF *keydef, uint keynr, ulong records,
 	  found++;
       }
       if (links > max_links) max_links=links;
+      hash_buckets_found++;
     }
   }
   if (found != records)
   {
-    DBUG_PRINT("error",("Found %ld of %ld records"));
+    DBUG_PRINT("error",("Found %ld of %ld records", found, records));
+    error=1;
+  }
+  if (keydef->hash_buckets != hash_buckets_found)
+  {
+    DBUG_PRINT("error",("Found %ld buckets, stats shows %ld buckets",
+                        hash_buckets_found, keydef->hash_buckets));
     error=1;
   }
   DBUG_PRINT("info",
-	     ("records: %ld   seeks: %d   max links: %d   hitrate: %.2f",
+	     ("records: %ld   seeks: %d   max links: %d   hitrate: %.2f   "
+              "buckets: %d",
 	      records,seek,max_links,
-	      (float) seek / (float) (records ? records : 1)));
+	      (float) seek / (float) (records ? records : 1), 
+              hash_buckets_found));
   if (print_status)
-    printf("Key: %d  records: %ld   seeks: %d   max links: %d   hitrate: %.2f\n",
+    printf("Key: %d  records: %ld   seeks: %d   max links: %d   "
+           "hitrate: %.2f   buckets: %d\n",
 	   keynr, records, seek, max_links,
-	   (float) seek / (float) (records ? records : 1));
+	   (float) seek / (float) (records ? records : 1), 
+           hash_buckets_found);
   return error;
 }
 
diff --git a/heap/hp_block.c b/heap/hp_block.c
index 6a022fb3084..f26b208b521 100644
--- a/heap/hp_block.c
+++ b/heap/hp_block.c
@@ -18,12 +18,19 @@
 
 #include "heapdef.h"
 
-	/* Find record according to record-position */
+/*
+  Find record according to record-position.
+      
+  The record is located by factoring position number pos into (p_0, p_1, ...) 
+  such that
+     pos = SUM_i(block->level_info[i].records_under_level * p_i)
+  {p_0, p_1, ...} serve as indexes to descend the blocks tree.
+*/
 
 byte *hp_find_block(HP_BLOCK *block, ulong pos)
 {
   reg1 int i;
-  reg3 HP_PTRS *ptr;
+  reg3 HP_PTRS *ptr; /* block base ptr */
 
   for (i=block->levels-1, ptr=block->root ; i > 0 ; i--)
   {
@@ -34,8 +41,18 @@ byte *hp_find_block(HP_BLOCK *block, ulong pos)
 }
 
 
-	/* get one new block-of-records. Alloc ptr to block if neaded */
-	/* Interrupts are stopped to allow ha_panic in interrupts */
+/*
+  Get one new block-of-records. Alloc ptr to block if needed
+  SYNOPSIS
+    hp_get_new_block()
+      block             HP_BLOCK tree-like block
+      alloc_length OUT  Amount of memory allocated from the heap
+      
+  Interrupts are stopped to allow ha_panic in interrupts 
+  RETURN
+    0  OK
+    1  Out of memory
+*/
 
 int hp_get_new_block(HP_BLOCK *block, ulong *alloc_length)
 {
@@ -46,6 +63,18 @@ int hp_get_new_block(HP_BLOCK *block, ulong *alloc_length)
     if (block->level_info[i].free_ptrs_in_block)
       break;
 
+  /*
+    Allocate space for leaf block plus space for upper level blocks up to
+    first level that has a free slot to put the pointer. 
+    In some cases we actually allocate more then we need:
+    Consider e.g. a situation where we have one level 1 block and one level 0
+    block, the level 0 block is full and this function is called. We only 
+    need a leaf block in this case. Nevertheless, we will get here with i=1 
+    and will also allocate sizeof(HP_PTRS) for non-leaf block and will never 
+    use this space.
+    This doesn't add much overhead - with current values of sizeof(HP_PTRS) 
+    and my_default_record_cache_size we get about 1/128 unused memory.
+   */
   *alloc_length=sizeof(HP_PTRS)*i+block->records_in_block* block->recbuffer;
   if (!(root=(HP_PTRS*) my_malloc(*alloc_length,MYF(0))))
     return 1;
@@ -60,21 +89,33 @@ int hp_get_new_block(HP_BLOCK *block, ulong *alloc_length)
     dont_break();		/* Dont allow SIGHUP or SIGINT */
     if ((uint) i == block->levels)
     {
+      /* Adding a new level on top of the existing ones. */
       block->levels=i+1;
+      /*
+        Use first allocated HP_PTRS as a top-level block. Put the current
+        block tree into the first slot of a new top-level block.
+      */
       block->level_info[i].free_ptrs_in_block=HP_PTRS_IN_NOD-1;
       ((HP_PTRS**) root)[0]= block->root;
       block->root=block->level_info[i].last_blocks= root++;
     }
+    /* Occupy the free slot we've found at level i */
     block->level_info[i].last_blocks->
       blocks[HP_PTRS_IN_NOD - block->level_info[i].free_ptrs_in_block--]=
 	(byte*) root;
-
+    
+    /* Add a block subtree with each node having one left-most child */
     for (j=i-1 ; j >0 ; j--)
     {
       block->level_info[j].last_blocks= root++;
       block->level_info[j].last_blocks->blocks[0]=(byte*) root;
       block->level_info[j].free_ptrs_in_block=HP_PTRS_IN_NOD-1;
     }
+    
+    /* 
+      root now points to last (block->records_in_block* block->recbuffer)
+      allocated bytes. Use it as a leaf block.
+    */
     block->level_info[0].last_blocks= root;
     allow_break();		/* Allow SIGHUP & SIGINT */
   }
diff --git a/heap/hp_clear.c b/heap/hp_clear.c
index 4440344f990..596d71ebe9c 100644
--- a/heap/hp_clear.c
+++ b/heap/hp_clear.c
@@ -97,6 +97,7 @@ void hp_clear_keys(HP_SHARE *info)
         VOID(hp_free_level(block,block->levels,block->root,(byte*) 0));
       block->levels=0;
       block->last_allocated=0;
+      keyinfo->hash_buckets= 0;
     }
   }
   info->index_length=0;
diff --git a/heap/hp_create.c b/heap/hp_create.c
index 02725576c8f..d1783118c0d 100644
--- a/heap/hp_create.c
+++ b/heap/hp_create.c
@@ -128,6 +128,7 @@ int heap_create(const char *name, uint keys, HP_KEYDEF *keydef,
 		   max_records);
 	keyinfo->delete_key= hp_delete_key;
 	keyinfo->write_key= hp_write_key;
+        keyinfo->hash_buckets= 0;
       }
     }
     share->min_records= min_records;
diff --git a/heap/hp_delete.c b/heap/hp_delete.c
index c918cf37f05..9cf8b8936b6 100644
--- a/heap/hp_delete.c
+++ b/heap/hp_delete.c
@@ -97,8 +97,8 @@ int hp_rb_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo,
     flag		Is set if we want's to correct info->current_ptr
 
   RETURN
-    0	ok
-    #	error number
+    0      Ok
+    other  Error code
 */
 
 int hp_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo,
@@ -151,6 +151,8 @@ int hp_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo,
     pos->ptr_to_rec=empty->ptr_to_rec;
     pos->next_key=empty->next_key;
   }
+  else
+    keyinfo->hash_buckets--;
 
   if (empty == lastpos)			/* deleted last hash key */
     DBUG_RETURN (0);
@@ -187,7 +189,11 @@ int hp_delete_key(HP_INFO *info, register HP_KEYDEF *keyinfo,
     }
     pos3= pos;				/* Link pos->next after lastpos */
   }
-  else pos3= 0;				/* Different positions merge */
+  else
+  {
+    pos3= 0;				/* Different positions merge */
+    keyinfo->hash_buckets--;
+  }
 
   empty[0]=lastpos[0];
   hp_movelink(pos3, empty, pos->next_key);
diff --git a/heap/hp_hash.c b/heap/hp_hash.c
index b54e76d79fe..ee5b4958e62 100644
--- a/heap/hp_hash.c
+++ b/heap/hp_hash.c
@@ -196,7 +196,18 @@ byte *hp_search_next(HP_INFO *info, HP_KEYDEF *keyinfo, const byte *key,
 }
 
 
-	/* Calculate pos according to keys */
+/*
+  Calculate position number for hash value.
+  SYNOPSIS
+    hp_mask()
+      hashnr     Hash value
+      buffmax    Value such that
+                 2^(n-1) < maxlength <= 2^n = buffmax
+      maxlength  
+  
+  RETURN
+    Array index, in [0..maxlength)
+*/
 
 ulong hp_mask(ulong hashnr, ulong buffmax, ulong maxlength)
 {
@@ -205,7 +216,12 @@ ulong hp_mask(ulong hashnr, ulong buffmax, ulong maxlength)
 }
 
 
-	/* Change link from pos to new_link */
+/*
+  Change
+    next_link -> ... -> X -> pos
+  to
+    next_link -> ... -> X -> newlink
+*/
 
 void hp_movelink(HASH_INFO *pos, HASH_INFO *next_link, HASH_INFO *newlink)
 {
diff --git a/heap/hp_write.c b/heap/hp_write.c
index 3b0ec76d616..43cee67b39c 100644
--- a/heap/hp_write.c
+++ b/heap/hp_write.c
@@ -36,7 +36,6 @@ int heap_write(HP_INFO *info, const byte *record)
   byte *pos;
   HP_SHARE *share=info->s;
   DBUG_ENTER("heap_write");
-
 #ifndef DBUG_OFF
   if (info->mode & O_RDONLY)
   {
@@ -160,7 +159,31 @@ static byte *next_free_record_pos(HP_SHARE *info)
 	      block_pos*info->block.recbuffer);
 }
 
-	/* Write a hash-key to the hash-index */
+
+/*
+  Write a hash-key to the hash-index
+  SYNOPSIS
+    info     Heap table info
+    keyinfo  Key info
+    record   Table record to added
+    recpos   Memory buffer where the table record will be stored if added 
+             successfully
+  NOTE
+    Hash index uses HP_BLOCK structure as a 'growable array' of HASH_INFO 
+    structs. Array size == number of entries in hash index.
+    hp_mask(hp_rec_hashnr()) maps hash entries values to hash array positions.
+    If there are several hash entries with the same hash array position P,
+    they are connected in a linked list via HASH_INFO::next_key. The first 
+    list element is located at position P, next elements are located at 
+    positions for which there is no record that should be located at that
+    position. The order of elements in the list is arbitrary.
+
+  RETURN
+    0  - OK
+    -1 - Out of memory
+    HA_ERR_FOUND_DUPP_KEY - Duplicate record on unique key. The record was 
+    still added and the caller must call hp_delete_key for it.
+*/
 
 int hp_write_key(HP_INFO *info, HP_KEYDEF *keyinfo,
 		 const byte *record, byte *recpos)
@@ -180,19 +203,54 @@ int hp_write_key(HP_INFO *info, HP_KEYDEF *keyinfo,
     DBUG_RETURN(-1);				/* No more memory */
   halfbuff= (long) share->blength >> 1;
   pos= hp_find_hash(&keyinfo->block,(first_index=share->records-halfbuff));
-
+  
+  /*
+    We're about to add one more hash array position, with hash_mask=#records.
+    The number of hash positions will change and some entries might need to 
+    be relocated to the newly added position. Those entries are currently 
+    members of the list that starts at #first_index position (this is 
+    guaranteed by properties of hp_mask(hp_rec_hashnr(X)) mapping function)
+    At #first_index position currently there may be either:
+    a) An entry with hashnr != first_index. We don't need to move it.
+    or
+    b) A list of items with hash_mask=first_index. The list contains entries
+       of 2 types:
+       1) entries that should be relocated to the list that starts at new 
+          position we're adding ('uppper' list)
+       2) entries that should be left in the list starting at #first_index 
+          position ('lower' list)
+  */
   if (pos != empty)				/* If some records */
   {
     do
     {
       hashnr = hp_rec_hashnr(keyinfo, pos->ptr_to_rec);
-      if (flag == 0)				/* First loop; Check if ok */
+      if (flag == 0)
+      {
+        /* 
+          First loop, bail out if we're dealing with case a) from above 
+          comment
+        */
 	if (hp_mask(hashnr, share->blength, share->records) != first_index)
 	  break;
+      }
+      /*
+        flag & LOWFIND - found a record that should be put into lower position
+        flag & LOWUSED - lower position occupied by the record
+        Same for HIGHFIND and HIGHUSED and 'upper' position
+
+        gpos  - ptr to last element in lower position's list
+        gpos2 - ptr to last element in upper position's list
+
+        ptr_to_rec - ptr to last entry that should go into lower list.
+        ptr_to_rec2 - same for upper list.
+      */
       if (!(hashnr & halfbuff))
-      {						/* Key will not move */
+      {						
+        /* Key should be put into 'lower' list */
 	if (!(flag & LOWFIND))
 	{
+          /* key is the first element to go into lower position */
 	  if (flag & HIGHFIND)
 	  {
 	    flag=LOWFIND | HIGHFIND;
@@ -203,16 +261,21 @@ int hp_write_key(HP_INFO *info, HP_KEYDEF *keyinfo,
 	  }
 	  else
 	  {
-	    flag=LOWFIND | LOWUSED;		/* key isn't changed */
+            /*
+              We can only get here at first iteration: key is at 'lower' 
+              position pos and should be left here.
+            */
+	    flag=LOWFIND | LOWUSED;
 	    gpos=pos;
 	    ptr_to_rec=pos->ptr_to_rec;
 	  }
 	}
 	else
-	{
+        {
+          /* Already have another key for lower position */
 	  if (!(flag & LOWUSED))
 	  {
-	    /* Change link of previous LOW-key */
+	    /* Change link of previous lower-list key */
 	    gpos->ptr_to_rec=ptr_to_rec;
 	    gpos->next_key=pos;
 	    flag= (flag & HIGHFIND) | (LOWFIND | LOWUSED);
@@ -222,19 +285,21 @@ int hp_write_key(HP_INFO *info, HP_KEYDEF *keyinfo,
 	}
       }
       else
-      {						/* key will be moved */
+      {
+        /* key will be put into 'higher' list */
 	if (!(flag & HIGHFIND))
 	{
 	  flag= (flag & LOWFIND) | HIGHFIND;
 	  /* key shall be moved to the last (empty) position */
-	  gpos2 = empty; empty=pos;
+	  gpos2= empty;
+          empty= pos;
 	  ptr_to_rec2=pos->ptr_to_rec;
 	}
 	else
 	{
 	  if (!(flag & HIGHUSED))
 	  {
-	    /* Change link of previous hash-key and save */
+	    /* Change link of previous upper-list key and save */
 	    gpos2->ptr_to_rec=ptr_to_rec2;
 	    gpos2->next_key=pos;
 	    flag= (flag & LOWFIND) | (HIGHFIND | HIGHUSED);
@@ -245,6 +310,15 @@ int hp_write_key(HP_INFO *info, HP_KEYDEF *keyinfo,
       }
     }
     while ((pos=pos->next_key));
+    
+    if ((flag & (LOWFIND | HIGHFIND)) == (LOWFIND | HIGHFIND))
+    {
+      /*
+        If both 'higher' and 'lower' list have at least one element, now
+        there are two hash buckets instead of one.
+      */
+      keyinfo->hash_buckets++;
+    }
 
     if ((flag & (LOWFIND | LOWUSED)) == LOWFIND)
     {
@@ -265,6 +339,7 @@ int hp_write_key(HP_INFO *info, HP_KEYDEF *keyinfo,
   {
     pos->ptr_to_rec=recpos;
     pos->next_key=0;
+    keyinfo->hash_buckets++;
   }
   else
   {
@@ -280,6 +355,7 @@ int hp_write_key(HP_INFO *info, HP_KEYDEF *keyinfo,
     }
     else
     {
+      keyinfo->hash_buckets++;
       pos->ptr_to_rec=recpos;
       pos->next_key=0;
       hp_movelink(pos, gpos, empty);