List:Commits« Previous MessageNext Message »
From:V Narayanan Date:July 24 2009 12:31pm
Subject:bzr commit into mysql-5.1-bugteam branch (v.narayanan:3039) Bug#45800
View as plain text  
#At file:///home/narayanan/Work/mysql_checkouts/shared_repository_directory/mysql-5.1-bugteam-45800-1/ based on revid:anurag.shekhar@stripped

 3039 V Narayanan	2009-07-24
      Bug#45800 crash when replacing into a merge table and there is a duplicate
      
      A REPLACE in the MERGE engine is actually a REPLACE
      into one (FIRST or LAST) of the underlying MyISAM
      tables. So in effect we have the meta data of the
      MERGE tables but we are inserting into a MyISAM table.
      
      When a REPLACE into a MERGE table (and the REPLACE
      conflicts with a duplicate in a child table) is done,
      we try to access the duplicate key information for
      the MERGE table. This information actually does not
      exist since it is the child tables that have keys
      and not the MERGE table. Hence this results in a crash.
      
      The problem can be resolved by modifying the MERGE
      engine to provide us the duplicate key information
      directly, instead of replying on the metadata on the
      server.
      
      The current patch modifies the MERGE engine to provide
      the duplicate key information to the server.
     @ include/myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Add a member to the st_mymerge_info structure that will
        store the duplicate key offset in the MERGE table. This
        offset will be the sum of the offset of the MyISAM table
        and the offset into the MERGE table.
     @ mysql-test/r/merge.result
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Result file for the test case.
     @ mysql-test/t/merge.test
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Added test case for both REPLACE and INSERT...ON DUPLICATE UPDATE.
     @ sql/handler.cc
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        With the current change the MERGE engine will be able to
        return the duplicate key offset and will not use MAX_KEY
        as a error indicator. This adversely affects the case
        when we do a INSERT into a MERGE table that has a duplicate.
        
        An INSERT into a MERGE table having a duplicate causes the
        write into the storage engine to fail. When we print an error
        using print_keydup_error we access the key information from
        the server, causing a crash.
        
        This problem can be resolved by checking for the case
        when table->key_info is NULL.
     @ storage/myisammrg/ha_myisammrg.cc
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        The info method now will process the HA_STATUS_ERRKEY flag
        and will return the index and the offset of the duplicate
        key.
        
        Since the storage engine returns the duplicate key information
        even if the key is on the child tables and not on the MERGE
        tables, we remove the code that sets the errkey to MAX_KEY.
     @ storage/myisammrg/ha_myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Set the HA_DUPLICATE_POS flag to indicate that the duplicate
        key information is now available in the MERGE storage engine.
     @ storage/myisammrg/myrg_info.c
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        We modify the myrg_status function to return the position of the
        duplicate key. The duplicate key position in the MERGE table will
        be the MyISAM file_offset and the offset within the MyISAM table
        where the key is located.

    modified:
      include/myisammrg.h
      mysql-test/r/merge.result
      mysql-test/t/merge.test
      sql/handler.cc
      storage/myisammrg/ha_myisammrg.cc
      storage/myisammrg/ha_myisammrg.h
      storage/myisammrg/myrg_info.c
=== modified file 'include/myisammrg.h'
--- a/include/myisammrg.h	2009-07-10 11:34:03 +0000
+++ b/include/myisammrg.h	2009-07-24 12:31:47 +0000
@@ -47,6 +47,7 @@ typedef struct st_mymerge_info		/* Struc
   ulonglong deleted;			/* Deleted records in database */
   ulonglong recpos;			/* Pos for last used record */
   ulonglong data_file_length;
+  ulonglong dupp_key_pos;               /* Offset of the Duplicate key in the merge table */
   uint	reclength;			/* Recordlength */
   int	errkey;				/* With key was dupplicated on err */
   uint	options;			/* HA_OPTION_... used */

=== modified file 'mysql-test/r/merge.result'
--- a/mysql-test/r/merge.result	2009-07-15 23:23:57 +0000
+++ b/mysql-test/r/merge.result	2009-07-24 12:31:47 +0000
@@ -2115,6 +2115,39 @@ insert into m1 (col1) values (1);
 insert into m1 (col1) values (1);
 ERROR 23000: Duplicate entry '' for key '*UNKNOWN*'
 drop table m1, t1;
+#
+# Bug#45800 crash when replacing into a merge table and there is a duplicate
+#
+# Replace duplicate value in child table when merge table doesn't have key
+# Create child table
+CREATE TABLE t1 (c1 INT PRIMARY KEY) ENGINE=MyISAM;
+# Create Merge table
+CREATE TABLE m1 (c1 INT NOT NULL) ENGINE=MRG_MyISAM INSERT_METHOD=LAST UNION=(t1);
+INSERT INTO m1  VALUES (666);
+SELECT * FROM m1;
+c1
+666
+# insert the duplicate value into the merge table
+REPLACE INTO m1 VALUES (666);
+SELECT * FROM m1;
+c1
+666
+DROP TABLE m1, t1;
+# Insert... on duplicate key update (with duplicate values in the table)
+# Create child table
+CREATE TABLE t1 (c1 INT PRIMARY KEY) ENGINE=MyISAM;
+# Create Merge table
+CREATE TABLE m1 (c1 INT NOT NULL) ENGINE=MRG_MyISAM INSERT_METHOD=LAST UNION=(t1);
+INSERT INTO m1  VALUES (666);
+SELECT * FROM m1;
+c1
+666
+# insert the duplicate value into the merge table
+INSERT INTO m1 VALUES (666) ON DUPLICATE KEY UPDATE c1=c1+1;
+SELECT * FROM m1;
+c1
+667
+DROP TABLE m1, t1;
 CREATE TABLE t1 (
 col1 INT(10)
 ) ENGINE=MyISAM  DEFAULT CHARSET=latin1;

=== modified file 'mysql-test/t/merge.test'
--- a/mysql-test/t/merge.test	2009-07-15 23:23:57 +0000
+++ b/mysql-test/t/merge.test	2009-07-24 12:31:47 +0000
@@ -1515,6 +1515,34 @@ insert into m1 (col1) values (1);
 
 drop table m1, t1;
 
+--echo #
+--echo # Bug#45800 crash when replacing into a merge table and there is a duplicate
+--echo #
+
+--echo # Replace duplicate value in child table when merge table doesn't have key
+--echo # Create child table
+CREATE TABLE t1 (c1 INT PRIMARY KEY) ENGINE=MyISAM;
+--echo # Create Merge table
+CREATE TABLE m1 (c1 INT NOT NULL) ENGINE=MRG_MyISAM INSERT_METHOD=LAST UNION=(t1);
+INSERT INTO m1  VALUES (666);
+SELECT * FROM m1;
+--echo # insert the duplicate value into the merge table
+REPLACE INTO m1 VALUES (666);
+SELECT * FROM m1;
+DROP TABLE m1, t1;
+
+--echo # Insert... on duplicate key update (with duplicate values in the table)
+--echo # Create child table
+CREATE TABLE t1 (c1 INT PRIMARY KEY) ENGINE=MyISAM;
+--echo # Create Merge table
+CREATE TABLE m1 (c1 INT NOT NULL) ENGINE=MRG_MyISAM INSERT_METHOD=LAST UNION=(t1);
+INSERT INTO m1  VALUES (666);
+SELECT * FROM m1;
+--echo # insert the duplicate value into the merge table
+INSERT INTO m1 VALUES (666) ON DUPLICATE KEY UPDATE c1=c1+1;
+SELECT * FROM m1;
+DROP TABLE m1, t1;
+
 #
 #Bug #44040   MySQL allows creating a MERGE table upon VIEWs but crashes 
 #when using it

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2009-07-24 10:11:23 +0000
+++ b/sql/handler.cc	2009-07-24 12:31:47 +0000
@@ -2552,9 +2552,9 @@ void handler::print_keydup_error(uint ke
   char key[MAX_KEY_LENGTH];
   String str(key,sizeof(key),system_charset_info);
 
-  if (key_nr == MAX_KEY)
+  if (key_nr == MAX_KEY || table->key_info == NULL)
   {
-    /* Key is unknown */
+    /* Key is unknown if key_nr is set to MAX_KEY or key_info is NULL */
     str.copy("", 0, system_charset_info);
     my_printf_error(ER_DUP_ENTRY, msg, MYF(0), str.c_ptr(), "*UNKNOWN*");
   }

=== modified file 'storage/myisammrg/ha_myisammrg.cc'
--- a/storage/myisammrg/ha_myisammrg.cc	2009-07-10 11:34:03 +0000
+++ b/storage/myisammrg/ha_myisammrg.cc	2009-07-24 12:31:47 +0000
@@ -874,16 +874,6 @@ int ha_myisammrg::info(uint flag)
     table->s->crashed= 1;
 #endif
   stats.data_file_length= mrg_info.data_file_length;
-  if (mrg_info.errkey >= (int) table_share->keys)
-  {
-    /*
-     If value of errkey is higher than the number of keys
-     on the table set errkey to MAX_KEY. This will be
-     treated as unknown key case and error message generator
-     won't try to locate key causing segmentation fault.
-    */
-    mrg_info.errkey= MAX_KEY;
-  }
   errkey= mrg_info.errkey;
   table->s->keys_in_use.set_prefix(table->s->keys);
   stats.mean_rec_length= mrg_info.reclength;
@@ -934,6 +924,17 @@ int ha_myisammrg::info(uint flag)
              min(file->keys, table->s->key_parts));
     }
   }
+  if (flag & HA_STATUS_ERRKEY)
+  {
+    /* 
+      Set the index of the key and the offset of the key that caused
+      the duplicate error. Here the mrg_info.dupp_key_pos will be the
+      sum of the offset of the myisam file and the offset into the
+      myisam file where the duplicate key is located.
+    */
+    errkey= mrg_info.errkey;
+    my_store_ptr(dup_ref, ref_length, mrg_info.dupp_key_pos);
+  }
   return 0;
 }
 

=== modified file 'storage/myisammrg/ha_myisammrg.h'
--- a/storage/myisammrg/ha_myisammrg.h	2009-02-12 11:12:07 +0000
+++ b/storage/myisammrg/ha_myisammrg.h	2009-07-24 12:31:47 +0000
@@ -44,7 +44,8 @@ class ha_myisammrg: public handler
 	    HA_NULL_IN_KEY | HA_CAN_INDEX_BLOBS | HA_FILE_BASED |
             HA_ANY_INDEX_MAY_BE_UNIQUE | HA_CAN_BIT_FIELD |
             HA_HAS_RECORDS |
-            HA_NO_COPY_ON_ALTER);
+            HA_NO_COPY_ON_ALTER |
+            HA_DUPLICATE_POS);
   }
   ulong index_flags(uint inx, uint part, bool all_parts) const
   {

=== modified file 'storage/myisammrg/myrg_info.c'
--- a/storage/myisammrg/myrg_info.c	2006-12-31 00:32:21 +0000
+++ b/storage/myisammrg/myrg_info.c	2009-07-24 12:31:47 +0000
@@ -58,9 +58,20 @@ int myrg_status(MYRG_INFO *info,register
     x->reclength= info->reclength;
     x->options= info->options;
     if (current_table)
+    {
       x->errkey= current_table->table->errkey;
+      /*
+        Calculate the position of the duplicate key to be the sum of the
+        offset of the myisam file and the offset into the file at which
+        the duplicate key is located.
+      */
+      x->dupp_key_pos= current_table->file_offset + current_table->table->dupp_key_pos;
+    }
     else
+    {
       x->errkey= 0;
+      x->dupp_key_pos= 0;
+    }
     x->rec_per_key = info->rec_per_key_part;
   }
   DBUG_RETURN(0);


Attachment: [text/bzr-bundle] bzr/v.narayanan@sun.com-20090724123147-al1fj8tyq0msr3h7.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (v.narayanan:3039) Bug#45800V Narayanan24 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (v.narayanan:3039)Bug#45800Ingo Strüwing24 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (v.narayanan:3039)Bug#45800V Narayanan27 Jul