List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:September 10 2010 9:52am
Subject:bzr commit into mysql-5.1-bugteam branch (mattias.jonsson:3506)
View as plain text  
#At file:///Users/mattiasj/mysql-bzr/topush-51-bugteam/ based on revid:mattias.jonsson@stripped

 3506 Mattias Jonsson	2010-09-10 [merge]
      merge

    modified:
      mysql-test/r/partition.result
      mysql-test/t/partition.test
      sql/ha_partition.cc
      sql/ha_partition.h
=== modified file 'mysql-test/r/partition.result'
--- a/mysql-test/r/partition.result	2010-08-27 08:43:51 +0000
+++ b/mysql-test/r/partition.result	2010-09-10 09:52:35 +0000
@@ -1,4 +1,29 @@
 drop table if exists t1, t2;
+#
+# Bug#55458: Partitioned MyISAM table gets crashed by multi-table update 
+#
+CREATE TABLE t1 (
+`id` int NOT NULL,
+`user_num` int DEFAULT NULL,
+PRIMARY KEY (`id`)
+) ENGINE=MyISAM CHARSET=latin1;
+INSERT INTO t1 VALUES (1,8601);
+INSERT INTO t1 VALUES (2,8601);
+INSERT INTO t1 VALUES (3,8601);
+INSERT INTO t1 VALUES (4,8601);
+CREATE TABLE t2 (
+`id` int(11) NOT NULL,
+`user_num` int DEFAULT NULL,
+`name` varchar(64) NOT NULL,
+PRIMARY KEY (`id`)
+) ENGINE=MyISAM CHARSET=latin1
+PARTITION BY HASH (id)
+PARTITIONS 2;
+INSERT INTO t2 VALUES (1,8601,'John');
+INSERT INTO t2 VALUES (2,8601,'JS');
+INSERT INTO t2 VALUES (3,8601,'John S');
+UPDATE t1, t2 SET t2.name = 'John Smith' WHERE t1.user_num = t2.user_num;
+DROP TABLE t1, t2;
 CREATE TABLE t1 (a INT, b INT)
 PARTITION BY LIST (a)
 SUBPARTITION BY HASH (b)

=== modified file 'mysql-test/t/partition.test'
--- a/mysql-test/t/partition.test	2010-03-22 12:30:27 +0000
+++ b/mysql-test/t/partition.test	2010-08-10 08:43:12 +0000
@@ -14,6 +14,33 @@
 drop table if exists t1, t2;
 --enable_warnings
 
+--echo #
+--echo # Bug#55458: Partitioned MyISAM table gets crashed by multi-table update 
+--echo #
+CREATE TABLE t1 (
+  `id` int NOT NULL,
+  `user_num` int DEFAULT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=MyISAM CHARSET=latin1;
+INSERT INTO t1 VALUES (1,8601);
+INSERT INTO t1 VALUES (2,8601);
+INSERT INTO t1 VALUES (3,8601);
+INSERT INTO t1 VALUES (4,8601);
+CREATE TABLE t2 (
+  `id` int(11) NOT NULL,
+  `user_num` int DEFAULT NULL,
+  `name` varchar(64) NOT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=MyISAM CHARSET=latin1
+PARTITION BY HASH (id)
+PARTITIONS 2;
+INSERT INTO t2 VALUES (1,8601,'John');
+INSERT INTO t2 VALUES (2,8601,'JS');
+INSERT INTO t2 VALUES (3,8601,'John S');
+
+UPDATE t1, t2 SET t2.name = 'John Smith' WHERE t1.user_num = t2.user_num;
+
+DROP TABLE t1, t2;
 #
 # Bug#48276: can't add column if subpartition exists
 CREATE TABLE t1 (a INT, b INT)

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2010-08-19 07:20:17 +0000
+++ b/sql/ha_partition.cc	2010-09-10 09:52:35 +0000
@@ -232,6 +232,8 @@ void ha_partition::init_handler_variable
   m_innodb= FALSE;
   m_extra_cache= FALSE;
   m_extra_cache_size= 0;
+  m_extra_prepare_for_update= FALSE;
+  m_extra_cache_part_id= NO_CURRENT_PART_ID;
   m_handler_status= handler_not_initialized;
   m_low_byte_first= 1;
   m_part_field_array= NULL;
@@ -5380,9 +5382,6 @@ void ha_partition::get_dynamic_partition
     when performing the sequential scan we will check this recorded value
     and call extra_opt whenever we start scanning a new partition.
 
-    monty: Neads to be fixed so that it's passed to all handlers when we
-    move to another partition during table scan.
-
   HA_EXTRA_NO_CACHE:
     When performing a UNION SELECT HA_EXTRA_NO_CACHE is called from the
     flush method in the select_union class.
@@ -5394,7 +5393,7 @@ void ha_partition::get_dynamic_partition
     for. If no cache is in use they will quickly return after finding
     this out. And we also ensure that all caches are disabled and no one
     is left by mistake.
-    In the future this call will probably be deleted an we will instead call
+    In the future this call will probably be deleted and we will instead call
     ::reset();
 
   HA_EXTRA_WRITE_CACHE:
@@ -5406,8 +5405,9 @@ void ha_partition::get_dynamic_partition
     This is called as part of a multi-table update. When the table to be
     updated is also scanned then this informs MyISAM handler to drop any
     caches if dynamic records are used (fixed size records do not care
-    about this call). We pass this along to all underlying MyISAM handlers
-    and ignore it for the rest.
+    about this call). We pass this along to the first partition to scan, and
+    flag that it is to be called after HA_EXTRA_CACHE when moving to the next
+    partition to scan.
 
   HA_EXTRA_PREPARE_FOR_DROP:
     Only used by MyISAM, called in preparation for a DROP TABLE.
@@ -5554,9 +5554,21 @@ int ha_partition::extra(enum ha_extra_fu
   case HA_EXTRA_PREPARE_FOR_RENAME:
     DBUG_RETURN(prepare_for_rename());
     break;
+  case HA_EXTRA_PREPARE_FOR_UPDATE:
+    DBUG_ASSERT(m_extra_cache);
+    /*
+      Needs to be run on the first partition in the range now, and 
+      later in late_extra_cache, when switching to a new partition to scan.
+    */
+    m_extra_prepare_for_update= TRUE;
+    if (m_part_spec.start_part != NO_CURRENT_PART_ID)
+    {
+      DBUG_ASSERT(m_extra_cache_part_id == m_part_spec.start_part);
+      VOID(m_file[m_part_spec.start_part]->extra(HA_EXTRA_PREPARE_FOR_UPDATE));
+    }
+    break;
   case HA_EXTRA_NORMAL:
   case HA_EXTRA_QUICK:
-  case HA_EXTRA_PREPARE_FOR_UPDATE:
   case HA_EXTRA_FORCE_REOPEN:
   case HA_EXTRA_PREPARE_FOR_DROP:
   case HA_EXTRA_FLUSH_CACHE:
@@ -5579,10 +5591,22 @@ int ha_partition::extra(enum ha_extra_fu
     break;
   }
   case HA_EXTRA_NO_CACHE:
+  {
+    int ret= 0;
+    if (m_extra_cache_part_id != NO_CURRENT_PART_ID)
+      ret= m_file[m_extra_cache_part_id]->extra(HA_EXTRA_NO_CACHE);
+    m_extra_cache= FALSE;
+    m_extra_cache_size= 0;
+    m_extra_prepare_for_update= FALSE;
+    m_extra_cache_part_id= NO_CURRENT_PART_ID;
+    DBUG_RETURN(ret);
+  }
   case HA_EXTRA_WRITE_CACHE:
   {
     m_extra_cache= FALSE;
     m_extra_cache_size= 0;
+    m_extra_prepare_for_update= FALSE;
+    m_extra_cache_part_id= NO_CURRENT_PART_ID;
     DBUG_RETURN(loop_extra(operation));
   }
   case HA_EXTRA_IGNORE_NO_KEY:
@@ -5710,6 +5734,7 @@ int ha_partition::extra_opt(enum ha_extr
 void ha_partition::prepare_extra_cache(uint cachesize)
 {
   DBUG_ENTER("ha_partition::prepare_extra_cache()");
+  DBUG_PRINT("info", ("cachesize %u", cachesize));
 
   m_extra_cache= TRUE;
   m_extra_cache_size= cachesize;
@@ -5768,16 +5793,18 @@ int ha_partition::loop_extra(enum ha_ext
 {
   int result= 0, tmp;
   handler **file;
+  bool is_select;
   DBUG_ENTER("ha_partition::loop_extra()");
   
-  /* 
-    TODO, 5.2: this is where you could possibly add optimisations to add the bitmap
-    _if_ a SELECT.
-  */
+  is_select= (thd_sql_command(ha_thd()) == SQLCOM_SELECT);
   for (file= m_file; *file; file++)
   {
-    if ((tmp= (*file)->extra(operation)))
-      result= tmp;
+    if (!is_select ||
+        bitmap_is_set(&(m_part_info->used_partitions), file - m_file))
+    {
+      if ((tmp= (*file)->extra(operation)))
+        result= tmp;
+    }
   }
   DBUG_RETURN(result);
 }
@@ -5798,14 +5825,22 @@ void ha_partition::late_extra_cache(uint
 {
   handler *file;
   DBUG_ENTER("ha_partition::late_extra_cache");
+  DBUG_PRINT("info", ("extra_cache %u partid %u size %u", m_extra_cache,
+                      partition_id, m_extra_cache_size));
 
-  if (!m_extra_cache)
+  if (!m_extra_cache && !m_extra_prepare_for_update)
     DBUG_VOID_RETURN;
   file= m_file[partition_id];
   if (m_extra_cache_size == 0)
     VOID(file->extra(HA_EXTRA_CACHE));
   else
     VOID(file->extra_opt(HA_EXTRA_CACHE, m_extra_cache_size));
+  if (m_extra_prepare_for_update)
+  {
+    DBUG_ASSERT(m_extra_cache);
+    VOID(file->extra(HA_EXTRA_PREPARE_FOR_UPDATE));
+  }
+  m_extra_cache_part_id= partition_id;
   DBUG_VOID_RETURN;
 }
 
@@ -5826,10 +5861,12 @@ void ha_partition::late_extra_no_cache(u
   handler *file;
   DBUG_ENTER("ha_partition::late_extra_no_cache");
 
-  if (!m_extra_cache)
+  if (!m_extra_cache && !m_extra_prepare_for_update)
     DBUG_VOID_RETURN;
   file= m_file[partition_id];
   VOID(file->extra(HA_EXTRA_NO_CACHE));
+  DBUG_ASSERT(partition_id == m_extra_cache_part_id);
+  m_extra_cache_part_id= NO_CURRENT_PART_ID;
   DBUG_VOID_RETURN;
 }
 

=== modified file 'sql/ha_partition.h'
--- a/sql/ha_partition.h	2010-07-09 13:00:33 +0000
+++ b/sql/ha_partition.h	2010-09-07 15:56:43 +0000
@@ -154,6 +154,10 @@ private:
   */
   bool m_extra_cache;
   uint m_extra_cache_size;
+  /* The same goes for HA_EXTRA_PREPARE_FOR_UPDATE */
+  bool m_extra_prepare_for_update;
+  /* Which partition has active cache */
+  uint m_extra_cache_part_id;
 
   void init_handler_variables();
   /*


Attachment: [text/bzr-bundle] bzr/mattias.jonsson@oracle.com-20100910095235-wtxh1fg2whh2hhal.bundle
Thread
bzr commit into mysql-5.1-bugteam branch (mattias.jonsson:3506) Mattias Jonsson10 Sep