List:Commits« Previous MessageNext Message »
From:V Narayanan Date:February 10 2009 8:34pm
Subject:bzr commit into mysql-5.1-bugteam branch (v.narayanan:2750) Bug#40675
View as plain text  
#At file:///home/narayanan/Work/mysql/W-M/mysql-5.1-bugteam/ based on revid:chad@stripped

 2750 V Narayanan	2009-02-11
      Bug #40675  MySQL 5.1 crash with index merge algorithm and Merge tables
      
      A Query in the MyISAM merge table was crashing 
      if the index merge algorithm was being used
      
      Index Merge optimization requires the reading of 
      multiple indexes at the same time. Reading multiple 
      indexes at once with current SE API means that we 
      need to have handler instance for each to-be-read 
      index. This is done by creating clones of the handlers 
      instances. The clone internally does a open of the handler.
      
      The open for a MERGE engine is handled in the following 
      phases
      
      1) open parent table
      2) generate list of underlying
         table
      3) attach underlying tables
      
      But the current implementation does only the first 
      phase (i.e.) open parent table.
      
      The current patch fixes this at the MERGE engine level, 
      by handling the clone operation within the MERGE engine 
      rather than in the storage engine API. It opens and 
      attaches the MyISAM tables on the MyISAM storage engine 
      interface directly within the MERGE engine. The new MyISAM 
      table instances, as well as the MERGE clone itself, are not 
      visible in the table cache. This is not a problem because 
      all locking is handled by the original MERGE table from which
      this is cloned of.
modified:
  mysql-test/r/merge.result
  mysql-test/t/merge.test
  storage/myisammrg/ha_myisammrg.cc
  storage/myisammrg/ha_myisammrg.h
  storage/myisammrg/myrg_open.c

per-file messages:
  mysql-test/r/merge.result
    updated the result file to reflect the new tests
    added to test the fix
  mysql-test/t/merge.test
    Added new tests to verify that the index merge
    algorithm does not crash in the merge engine.
  storage/myisammrg/ha_myisammrg.cc
    Implement the clone method, that handles
    
    1) Cloning the handler
    2) Opening underlying MYISAM child tables
    3) Copies the state of the original handler and the children into
       the cloned instances
    4) Sets the appropriate flags
  storage/myisammrg/ha_myisammrg.h
    Added a flag that is set to indicate that the current 
    instance is cloned. Also added the prototype or the clone 
    method.
  storage/myisammrg/myrg_open.c
    Since we do now again use myrg_open() in the server
    removed the comments marking this as deadcode.
=== modified file 'mysql-test/r/merge.result'
--- a/mysql-test/r/merge.result	2008-10-09 08:55:16 +0000
+++ b/mysql-test/r/merge.result	2009-02-10 20:34:38 +0000
@@ -2014,7 +2014,6 @@ TABLE_SCHEMA = 'test' and TABLE_NAME='tm
 TABLE_CATALOG	TABLE_SCHEMA	TABLE_NAME	TABLE_TYPE	ENGINE	VERSION	ROW_FORMAT	TABLE_ROWS	AVG_ROW_LENGTH	DATA_LENGTH	MAX_DATA_LENGTH	INDEX_LENGTH	DATA_FREE	AUTO_INCREMENT	CREATE_TIME	UPDATE_TIME	CHECK_TIME	TABLE_COLLATION	CHECKSUM	CREATE_OPTIONS	TABLE_COMMENT
 NULL	test	tm1	BASE TABLE	NULL	NULL	NULL	#	#	#	#	#	#	#	#	#	#	NULL	#	#	Unable to open underlying table which is differently defined or of non-MyISAM ty
 DROP TABLE tm1;
-End of 5.1 tests
 CREATE TABLE t1(C1 INT, C2 INT, KEY C1(C1), KEY C2(C2)) ENGINE=MYISAM;
 CREATE TABLE t2(C1 INT, C2 INT, KEY C1(C1), KEY C2(C2)) ENGINE=MYISAM;
 CREATE TABLE t3(C1 INT, C2 INT, KEY C1(C1), KEY C2(C2)) ENGINE=MYISAM;
@@ -2030,4 +2029,48 @@ EXPLAIN SELECT COUNT(*) FROM t4;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Select tables optimized away
 DROP TABLE t1, t2, t3, t4;
+#
+# Bug #40675 MySQL 5.1 crash with index merge algorithm and Merge tables
+#
+# create MYISAM table t1 and insert values into it
+CREATE TABLE t1(a INT);
+INSERT INTO t1 VALUES(1);
+# create MYISAM table t2 and insert values into it
+CREATE TABLE t2(a INT, b INT, dummy CHAR(16) DEFAULT '', KEY(a), KEY(b));
+INSERT INTO t2(a,b) VALUES
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(1,2);
+# Create the merge table t3
+CREATE TABLE t3(a INT, b INT, dummy CHAR(16) DEFAULT '', KEY(a), KEY(b))
+ENGINE=MERGE UNION=(t2) INSERT_METHOD=FIRST;
+# Lock tables t1 and t3 for write
+LOCK TABLES t1 WRITE, t3 WRITE;
+# Insert values into the merge table t3
+INSERT INTO t3(a,b) VALUES(1,2);
+# select from the join of t2 and t3 (The merge table)
+SELECT t3.a FROM t1,t3 WHERE t3.b=2 AND t3.a=1;
+a
+1
+1
+# Unlock the tables
+UNLOCK TABLES;
+# drop the created tables
+DROP TABLE t1, t2, t3;
 End of 5.1 tests

=== modified file 'mysql-test/t/merge.test'
--- a/mysql-test/t/merge.test	2008-10-09 08:55:16 +0000
+++ b/mysql-test/t/merge.test	2009-02-10 20:34:38 +0000
@@ -1405,8 +1405,6 @@ TABLE_SCHEMA = 'test' and TABLE_NAME='tm
 
 DROP TABLE tm1;
 
---echo End of 5.1 tests
-
 #
 # Bug#36006 - Optimizer does table scan for select count(*)
 #
@@ -1422,4 +1420,54 @@ EXPLAIN SELECT COUNT(*) FROM t1;
 EXPLAIN SELECT COUNT(*) FROM t4;
 DROP TABLE t1, t2, t3, t4;
 
+--echo #
+--echo # Bug #40675 MySQL 5.1 crash with index merge algorithm and Merge tables
+--echo #
+
+--echo # create MYISAM table t1 and insert values into it
+CREATE TABLE t1(a INT);
+INSERT INTO t1 VALUES(1);
+
+--echo # create MYISAM table t2 and insert values into it
+CREATE TABLE t2(a INT, b INT, dummy CHAR(16) DEFAULT '', KEY(a), KEY(b));
+INSERT INTO t2(a,b) VALUES
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),(0,0),
+(1,2);
+
+--echo # Create the merge table t3
+CREATE TABLE t3(a INT, b INT, dummy CHAR(16) DEFAULT '', KEY(a), KEY(b))
+ENGINE=MERGE UNION=(t2) INSERT_METHOD=FIRST;
+
+--echo # Lock tables t1 and t3 for write
+LOCK TABLES t1 WRITE, t3 WRITE;
+
+--echo # Insert values into the merge table t3
+INSERT INTO t3(a,b) VALUES(1,2);
+
+--echo # select from the join of t2 and t3 (The merge table)
+SELECT t3.a FROM t1,t3 WHERE t3.b=2 AND t3.a=1;
+
+--echo # Unlock the tables
+UNLOCK TABLES;
+
+--echo # drop the created tables
+DROP TABLE t1, t2, t3;
+
 --echo End of 5.1 tests

=== modified file 'storage/myisammrg/ha_myisammrg.cc'
--- a/storage/myisammrg/ha_myisammrg.cc	2008-04-25 21:45:58 +0000
+++ b/storage/myisammrg/ha_myisammrg.cc	2009-02-10 20:34:38 +0000
@@ -116,7 +116,7 @@ static handler *myisammrg_create_handler
 */
 
 ha_myisammrg::ha_myisammrg(handlerton *hton, TABLE_SHARE *table_arg)
-  :handler(hton, table_arg), file(0)
+  :handler(hton, table_arg), file(0), is_cloned(0)
 {}
 
 
@@ -413,7 +413,11 @@ int ha_myisammrg::open(const char *name,
 
   /* retrieve children table list. */
   my_errno= 0;
-  if (!(file= myrg_parent_open(name, myisammrg_parent_open_callback, this)))
+  if (is_cloned)
+  {
+    info(HA_STATUS_NO_LOCK | HA_STATUS_VARIABLE | HA_STATUS_CONST);
+  }
+  else if (!(file= myrg_parent_open(name, myisammrg_parent_open_callback, this)))
   {
     DBUG_PRINT("error", ("my_errno %d", my_errno));
     DBUG_RETURN(my_errno ? my_errno : -1);
@@ -422,6 +426,70 @@ int ha_myisammrg::open(const char *name,
   DBUG_RETURN(0);
 }
 
+/**
+ * Returns a cloned instance of the current handler.
+ *
+ * @return A cloned handler instance.
+ */
+handler *ha_myisammrg::clone(MEM_ROOT *mem_root)
+{
+  MYRG_TABLE    *u_table,*newu_table;
+  ha_myisammrg *new_handler= 
+    (ha_myisammrg*) get_new_handler(table->s, mem_root, table->s->db_type());
+  if (!new_handler)
+    return NULL;
+  
+  /* Inform ha_myisammrg::open() that it is a cloned handler */
+  new_handler->is_cloned= TRUE;
+  /*
+    Allocate handler->ref here because otherwise ha_open will allocate it
+    on this->table->mem_root and we will not be able to reclaim that memory 
+    when the clone handler object is destroyed.
+  */
+  if (!(new_handler->ref= (uchar*) alloc_root(mem_root, ALIGN_SIZE(ref_length)*2)))
+  {
+    delete new_handler;
+    return NULL;
+  }
+
+  /*
+    Open and attaches the MyISAM tables,that are under the MERGE table 
+    parent, on the MyISAM storage engine interface directly within the
+    MERGE engine. The new MyISAM table instances, as well as the MERGE 
+    clone itself, are not visible in the table cache. This is not a 
+    problem because all locking is handled by the original MERGE table
+    from which this is cloned of.
+  */
+  if (!(new_handler->file= myrg_open(table->s->normalized_path.str, table->db_stat, 
+                                     HA_OPEN_IGNORE_IF_LOCKED)))
+  {
+    delete new_handler;
+    return NULL;
+  }
+
+  /*
+    Iterate through the original child tables and
+    copy the state into the cloned child tables.
+    We need to do this because all the child tables
+    can be involved in delete.
+  */
+  newu_table= new_handler->file->open_tables;
+  for (u_table= file->open_tables; u_table < file->end_table; u_table++)
+  {
+    newu_table->table->state= u_table->table->state;
+    newu_table++;
+  }
+
+  file->children_attached= TRUE;
+
+  if (!new_handler->ha_open(table, table->s->normalized_path.str, table->db_stat,
+                            HA_OPEN_IGNORE_IF_LOCKED))
+    return new_handler;
+  
+  delete new_handler;
+  return NULL;
+ }
+
 
 /**
   @brief Attach children to a MERGE table.
@@ -613,9 +681,10 @@ int ha_myisammrg::close(void)
   DBUG_ENTER("ha_myisammrg::close");
   /*
     Children must not be attached here. Unless the MERGE table has no
-    children. In this case children_attached is always true.
+    children or the handler instance has been cloned. In these cases 
+    children_attached is always true. 
   */
-  DBUG_ASSERT(!this->file->children_attached || !this->file->tables);
+  DBUG_ASSERT(!this->file->children_attached || !this->file->tables || this->is_cloned);
   rc= myrg_close(file);
   file= 0;
   DBUG_RETURN(rc);

=== modified file 'storage/myisammrg/ha_myisammrg.h'
--- a/storage/myisammrg/ha_myisammrg.h	2008-04-25 21:45:58 +0000
+++ b/storage/myisammrg/ha_myisammrg.h	2009-02-10 20:34:38 +0000
@@ -25,6 +25,7 @@
 class ha_myisammrg: public handler
 {
   MYRG_INFO *file;
+  my_bool is_cloned;                    /* This instance has been cloned */
 
  public:
   TABLE_LIST    *next_child_attach;     /* next child to attach */
@@ -60,6 +61,7 @@ class ha_myisammrg: public handler
   int open(const char *name, int mode, uint test_if_locked);
   int attach_children(void);
   int detach_children(void);
+  virtual handler *clone(MEM_ROOT *mem_root);
   int close(void);
   int write_row(uchar * buf);
   int update_row(const uchar * old_data, uchar * new_data);

=== modified file 'storage/myisammrg/myrg_open.c'
--- a/storage/myisammrg/myrg_open.c	2007-12-11 14:32:10 +0000
+++ b/storage/myisammrg/myrg_open.c	2009-02-10 20:34:38 +0000
@@ -33,7 +33,6 @@
         myrg_attach_children(). Please duplicate changes in these
         functions or make common sub-functions.
 */
-/* purecov: begin deadcode */ /* not used in MySQL server */
 
 MYRG_INFO *myrg_open(const char *name, int mode, int handle_locking)
 {
@@ -195,7 +194,6 @@ err:
   my_errno=save_errno;
   DBUG_RETURN (NULL);
 }
-/* purecov: end */
 
 
 /**

Thread
bzr commit into mysql-5.1-bugteam branch (v.narayanan:2750) Bug#40675V Narayanan10 Feb
Re: bzr commit into mysql-5.1-bugteam branch (v.narayanan:2750)Bug#40675V Narayanan10 Feb