#At file:///home/narayanan/Work/mysql/W-M/mysql-5.1-bugteam/
2750 V Narayanan 2009-02-06
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.
modified:
mysql-test/r/merge.result
mysql-test/t/merge.test
storage/myisammrg/ha_myisammrg.cc
storage/myisammrg/ha_myisammrg.h
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.
=== 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-06 12:45:37 +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,54 @@ 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
+#
+# Drop tables used in test if any
+DROP TABLE IF EXISTS t1, t2, t3;
+Warnings:
+Note 1051 Unknown table 't1'
+Note 1051 Unknown table 't2'
+Note 1051 Unknown table 't3'
+# 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-06 12:45:37 +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,57 @@ 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 # Drop tables used in test if any
+DROP TABLE IF EXISTS t1, t2, t3;
+
+--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-06 12:45:37 +0000
@@ -117,7 +117,10 @@ static handler *myisammrg_create_handler
ha_myisammrg::ha_myisammrg(handlerton *hton, TABLE_SHARE *table_arg)
:handler(hton, table_arg), file(0)
-{}
+{
+ /* set the instance to not cloned by default. */
+ is_cloned= 0;
+}
/**
@@ -413,7 +416,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 +429,59 @@ 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)))
+ return NULL;
+
+ /*
+ Open the MySQL tables that are under the MERGE table
+ parent
+ */
+ if (!(new_handler->file= myrg_open(table->s->normalized_path.str, table->db_stat, HA_OPEN_IGNORE_IF_LOCKED)))
+ {
+ 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;
+ return NULL;
+ }
+
/**
@brief Attach children to a MERGE table.
@@ -613,9 +673,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-06 12:45:37 +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);