List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:January 24 2011 3:47pm
Subject:bzr commit into mysql-5.5 branch (mattias.jonsson:3271) Bug#59316
View as plain text  
#At file:///Users/mattiasj/mysql-bzr/b59316-55_2/ based on revid:martin.hansson@stripped

 3271 Mattias Jonsson	2011-01-24
      Bug#59316: Partitioning and index_merge memory leak
      
      When executing row-ordered-retrieval index merge,
      the handler was cloned, but it used the wrong
      memory root, so instead of allocating memory
      on the thread/query's mem_root, it used the table's
      mem_root, resulting in non released memory in the
      table object, and was not freed until the table was
      closed.
      
      Solution was to ensure that memory used during cloning
      of a handler was allocated from the correct memory root.
     @ sql/ha_partition.cc
        Created a new ha_partition constructor for use when
        cloning.
        Also set the partitions handlers to be clones of the
        original tables partitions.
        Moved code from get_from_handler_file to a new function
        read_handler_file.
     @ sql/ha_partition.h
        Moved is_clone variable to handler.h.
        Added helper function get_handler_for_partition.
     @ sql/handler.cc
        Moved allocation of ref to ha_open, also for
        clones.
     @ sql/handler.h
        Added is_clone_of and clone_mem_root.
        For more generic handling of clones
     @ storage/heap/ha_heap.cc
        Removed ha_heap::clone(), using handler::clone() instead.
        Moved clone specific code to ::open() instead.
     @ storage/heap/ha_heap.h
        Removed clone, added helper function hp_info().
     @ storage/myisam/ha_myisam.cc
        Removed clone(), using handler::clone() instead.
        Moved clone specific code to ::open() instead.
     @ storage/myisam/ha_myisam.h
        removed clone().
     @ storage/myisammrg/ha_myisammrg.cc
        Moved is_cloned variable to handler::is_clone_of.
        Removed ::clone().
        Moved clone specific code to ::open().
     @ storage/myisammrg/ha_myisammrg.h
        Moved is_cloned to handler::is_clone_of.
        removed ::clone(), using handler::clone instead.

    modified:
      sql/ha_partition.cc
      sql/ha_partition.h
      sql/handler.cc
      sql/handler.h
      storage/heap/ha_heap.cc
      storage/heap/ha_heap.h
      storage/myisam/ha_myisam.cc
      storage/myisam/ha_myisam.h
      storage/myisammrg/ha_myisammrg.cc
      storage/myisammrg/ha_myisammrg.h
=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2010-12-17 20:58:40 +0000
+++ b/sql/ha_partition.cc	2011-01-24 15:47:28 +0000
@@ -150,15 +150,13 @@ static uint alter_table_flags(uint flags
 
 const uint ha_partition::NO_CURRENT_PART_ID= 0xFFFFFFFF;
 
-/*
-  Constructor method
+/**
+  ha_partition constructor method
 
-  SYNOPSIS
-    ha_partition()
-    table                       Table object
+    @param hton           Handlerton (partition_hton)
+    @param share          Table share object
 
-  RETURN VALUE
-    NONE
+  @return New partition handler
 */
 
 ha_partition::ha_partition(handlerton *hton, TABLE_SHARE *share)
@@ -171,15 +169,40 @@ ha_partition::ha_partition(handlerton *h
 }
 
 
-/*
-  Constructor method
+/**
+  ha_partition constructor method used by ha_partition::clone()
 
-  SYNOPSIS
-    ha_partition()
-    part_info                       Partition info
+    @param hton               Handlerton (partition_hton)
+    @param share              Table share object
+    @param part_info_arg      partition_info to use
+    @param clone_arg          ha_partition to clone
+    @param clme_mem_root_arg  MEM_ROOT to use
+
+  @return New partition handler
+*/
+
+ha_partition::ha_partition(handlerton *hton, TABLE_SHARE *share,
+                           partition_info *part_info_arg,
+                           handler *clone_arg,
+                           MEM_ROOT *clone_mem_root_arg)
+  :handler(hton, share), m_part_info(part_info_arg), m_create_handler(TRUE),
+   m_is_sub_partitioned(m_part_info->is_sub_partitioned())
+{
+  DBUG_ENTER("ha_partition::ha_partition(clone)");
+  is_clone_of= clone_arg;
+  clone_mem_root= clone_mem_root_arg;
+  init_handler_variables();
+  DBUG_VOID_RETURN;
+}
 
-  RETURN VALUE
-    NONE
+
+/**
+  ha_partition constructor method
+
+    @param hton           Handlerton (partition_hton)
+    @param part_info      partition_info to use
+
+  @return New partition handler
 */
 
 ha_partition::ha_partition(handlerton *hton, partition_info *part_info)
@@ -187,8 +210,8 @@ ha_partition::ha_partition(handlerton *h
    m_is_sub_partitioned(m_part_info->is_sub_partitioned())
 {
   DBUG_ENTER("ha_partition::ha_partition(part_info)");
-  init_handler_variables();
   DBUG_ASSERT(m_part_info);
+  init_handler_variables();
   DBUG_VOID_RETURN;
 }
 
@@ -243,7 +266,6 @@ void ha_partition::init_handler_variable
   m_rec0= 0;
   m_curr_key_info[0]= NULL;
   m_curr_key_info[1]= NULL;
-  is_clone= FALSE,
   m_part_func_monotonicity_info= NON_MONOTONIC;
   auto_increment_lock= FALSE;
   auto_increment_safe_stmt_log_lock= FALSE;
@@ -2142,6 +2164,7 @@ bool ha_partition::create_handler_file(c
   DBUG_PRINT("info", ("table name = %s, num_parts = %u", name,
                       num_parts));
   tot_name_len= 0;
+  /* Calculate the total storage needed for all partitioning names */
   for (i= 0; i < num_parts; i++)
   {
     part_elem= part_it++;
@@ -2191,7 +2214,7 @@ bool ha_partition::create_handler_file(c
   tot_len_byte= 4 * tot_len_words;
   if (!(file_buffer= (uchar *) my_malloc(tot_len_byte, MYF(MY_ZEROFILL))))
     DBUG_RETURN(TRUE);
-  engine_array= (file_buffer + 12);
+  engine_array= (file_buffer + PAR_FILE_DB_TYPE_OFFSET);
   name_buffer_ptr= (char*) (file_buffer + ((4 + tot_partition_words) * 4));
   part_it.rewind();
   for (i= 0; i < num_parts; i++)
@@ -2231,7 +2254,8 @@ bool ha_partition::create_handler_file(c
   chksum= 0;
   int4store(file_buffer, tot_len_words);
   int4store(file_buffer + 8, tot_parts);
-  int4store(file_buffer + 12 + (tot_partition_words * 4), tot_name_len);
+  int4store(file_buffer + PAR_FILE_DB_TYPE_OFFSET + (tot_partition_words * 4),
+            tot_name_len);
   for (i= 0; i < tot_len_words; i++)
     chksum^= uint4korr(file_buffer + 4 * i);
   int4store(file_buffer + 4, chksum);
@@ -2269,10 +2293,19 @@ void ha_partition::clear_handler_file()
 {
   if (m_engine_array)
     plugin_unlock_list(NULL, m_engine_array, m_tot_parts);
-  my_free(m_file_buffer);
-  my_free(m_engine_array);
-  m_file_buffer= NULL;
-  m_engine_array= NULL;
+  DBUG_PRINT("info", ("m_file_buffer %p m_name_buffer_ptr %p free+file->NULL",
+                      m_file_buffer, m_name_buffer_ptr));
+  if (m_file_buffer)
+  {
+    my_free(m_file_buffer);
+    m_file_buffer= NULL;
+    m_name_buffer_ptr= NULL;
+  }
+  if (m_engine_array)
+  {
+    my_free(m_engine_array);
+    m_engine_array= NULL;
+  }
 }
 
 /*
@@ -2294,6 +2327,9 @@ bool ha_partition::create_handlers(MEM_R
   handlerton *hton0;
   DBUG_ENTER("create_handlers");
 
+  DBUG_PRINT("info", ("mem_root %p thd: %p table: %p",
+                      mem_root, ha_thd()->mem_root, &table->mem_root));
+  DBUG_ASSERT(!is_clone_of);
   if (!(m_file= (handler **) alloc_root(mem_root, alloc_len)))
     DBUG_RETURN(TRUE);
   m_file_tot_parts= m_tot_parts;
@@ -2301,8 +2337,7 @@ bool ha_partition::create_handlers(MEM_R
   for (i= 0; i < m_tot_parts; i++)
   {
     handlerton *hton= plugin_data(m_engine_array[i], handlerton*);
-    if (!(m_file[i]= get_new_handler(table_share, mem_root,
-                                     hton)))
+    if (!(m_file[i]= get_new_handler(table_share, mem_root, hton)))
       DBUG_RETURN(TRUE);
     DBUG_PRINT("info", ("engine_type: %u", hton->db_type));
   }
@@ -2342,6 +2377,8 @@ bool ha_partition::new_handlers_from_par
   List_iterator_fast <partition_element> part_it(m_part_info->partitions);
   DBUG_ENTER("ha_partition::new_handlers_from_part_info");
 
+  DBUG_PRINT("info", ("mem_root %p thd: %p table: %p",
+                      mem_root, ha_thd()->mem_root, &table->mem_root));
   if (!(m_file= (handler **) alloc_root(mem_root, alloc_len)))
   {
     mem_alloc_error(alloc_len);
@@ -2364,20 +2401,38 @@ bool ha_partition::new_handlers_from_par
     {
       for (j= 0; j < m_part_info->num_subparts; j++)
       {
-	if (!(m_file[part_count++]= get_new_handler(table_share, mem_root,
-                                                    part_elem->engine_type)))
+	if (!(m_file[part_count]= get_new_handler(table_share, mem_root,
+                                                  part_elem->engine_type)))
           goto error;
+        if (is_clone_of)
+        {
+          uint i= part_count;
+          DBUG_ASSERT(clone_mem_root == mem_root);
+          ha_partition *file= (ha_partition*) is_clone_of;
+          m_file[i]->is_clone_of= file->get_handler_for_partition(i);
+          m_file[i]->clone_mem_root= clone_mem_root;
+        }
+        part_count++;
 	DBUG_PRINT("info", ("engine_type: %u",
                    (uint) ha_legacy_type(part_elem->engine_type)));
       }
     }
     else
     {
-      if (!(m_file[part_count++]= get_new_handler(table_share, mem_root,
-                                                  part_elem->engine_type)))
+      if (!(m_file[part_count]= get_new_handler(table_share, mem_root,
+                                                part_elem->engine_type)))
         goto error;
       DBUG_PRINT("info", ("engine_type: %u",
                  (uint) ha_legacy_type(part_elem->engine_type)));
+      if (is_clone_of)
+      {
+        uint i= part_count;
+        DBUG_ASSERT(clone_mem_root == mem_root);
+        ha_partition *file= (ha_partition*) is_clone_of;
+        m_file[i]->is_clone_of= file->get_handler_for_partition(i);
+        m_file[i]->clone_mem_root= clone_mem_root;
+      }
+      part_count++;
     }
   } while (++i < m_part_info->num_parts);
   if (part_elem->engine_type == myisam_hton)
@@ -2393,35 +2448,15 @@ error_end:
 }
 
 
-/*
-  Get info about partition engines and their names from the .par file
-
-  SYNOPSIS
-    get_from_handler_file()
-    name                        Full path of table name
-    mem_root			Allocate memory through this
-
-  RETURN VALUE
-    TRUE                        Error
-    FALSE                       Success
-
-  DESCRIPTION
-    Open handler file to get partition names, engine types and number of
-    partitions.
-*/
-
-bool ha_partition::get_from_handler_file(const char *name, MEM_ROOT *mem_root)
+bool ha_partition::read_handler_file(const char *name)
 {
   char buff[FN_REFLEN], *address_tot_name_len;
   File file;
-  char *file_buffer, *name_buffer_ptr;
-  handlerton **engine_array;
+  char *file_buffer;
   uint i, len_bytes, len_words, tot_partition_words, tot_name_words, chksum;
-  DBUG_ENTER("ha_partition::get_from_handler_file");
+  DBUG_ENTER("ha_partition::read_handler_file");
   DBUG_PRINT("enter", ("table name: '%s'", name));
 
-  if (m_file_buffer)
-    DBUG_RETURN(FALSE);
   fn_format(buff, name, "", ha_par_ext, MY_APPEND_EXT);
 
   /* Following could be done with mysql_file_stat to read in whole file */
@@ -2438,6 +2473,7 @@ bool ha_partition::get_from_handler_file
   if (mysql_file_read(file, (uchar *) file_buffer, len_bytes, MYF(MY_NABP)))
     goto err2;
 
+  (void) mysql_file_close(file, MYF(0));
   chksum= 0;
   for (i= 0; i < len_words; i++)
     chksum ^= uint4korr((file_buffer) + 4 * i);
@@ -2446,48 +2482,90 @@ bool ha_partition::get_from_handler_file
   m_tot_parts= uint4korr((file_buffer) + 8);
   DBUG_PRINT("info", ("No of parts = %u", m_tot_parts));
   tot_partition_words= (m_tot_parts + 3) / 4;
-  engine_array= (handlerton **) my_alloca(m_tot_parts * sizeof(handlerton*));
-  for (i= 0; i < m_tot_parts; i++)
-  {
-    engine_array[i]= ha_resolve_by_legacy_type(ha_thd(),
-                                               (enum legacy_db_type)
-                                               *(uchar *) ((file_buffer) +
-                                                           12 + i));
-    if (!engine_array[i])
-      goto err3;
-  }
-  address_tot_name_len= file_buffer + 12 + 4 * tot_partition_words;
+  address_tot_name_len= file_buffer + PAR_FILE_DB_TYPE_OFFSET +
+                          4 * tot_partition_words;
   tot_name_words= (uint4korr(address_tot_name_len) + 3) / 4;
   if (len_words != (tot_partition_words + tot_name_words + 4))
-    goto err3;
-  name_buffer_ptr= file_buffer + 16 + 4 * tot_partition_words;
-  (void) mysql_file_close(file, MYF(0));
+    goto err2;
+  m_name_buffer_ptr= file_buffer + 16 + 4 * tot_partition_words;
   m_file_buffer= file_buffer;          // Will be freed in clear_handler_file()
-  m_name_buffer_ptr= name_buffer_ptr;
-  
-  if (!(m_engine_array= (plugin_ref*)
-                my_malloc(m_tot_parts * sizeof(plugin_ref), MYF(MY_WME))))
-    goto err3;
+  DBUG_RETURN(FALSE);
+err2:
+  my_free(file_buffer);
+  DBUG_RETURN(TRUE);
+err1:
+  (void) mysql_file_close(file, MYF(0));
+  DBUG_RETURN(TRUE);
+}
 
-  for (i= 0; i < m_tot_parts; i++)
-    m_engine_array[i]= ha_lock_engine(NULL, engine_array[i]);
+/*
+  Get info about partition engines and their names from the .par file
+
+  SYNOPSIS
+    get_from_handler_file()
+    name                        Full path of table name
+    mem_root			Allocate memory through this
 
-  my_afree((gptr) engine_array);
+  RETURN VALUE
+    TRUE                        Error
+    FALSE                       Success
+
+  DESCRIPTION
+    Open handler file to get partition names, engine types and number of
+    partitions.
+*/
+bool ha_partition::get_from_handler_file(const char *name, MEM_ROOT *mem_root)
+{
+  DBUG_ENTER("ha_partition::get_from_handler_file");
+
+  if (!m_file_buffer && !m_name_buffer_ptr)
+  {
+    if (read_handler_file(name))
+      DBUG_RETURN(TRUE);
+  }
+  DBUG_ASSERT(m_file_buffer && m_name_buffer_ptr);
+  /*
+    If m_file is already created, only the names was needed. This happens
+    during ::clone() in ::open().
+    TODO: Save the partitions names in the table_share for easier access.
+  */
+  if (m_file)
+    DBUG_RETURN(FALSE);
+
+  DBUG_ASSERT(!m_engine_array);
+  if (!m_engine_array)
+  {
+    enum legacy_db_type first_db_type, db_type;
+    uint i;
+    handlerton *first_engine;
+    first_db_type= (enum legacy_db_type) m_file_buffer[PAR_FILE_DB_TYPE_OFFSET];
+    first_engine= ha_resolve_by_legacy_type(ha_thd(), first_db_type);
+    if (!first_engine)
+      DBUG_RETURN(TRUE);
     
-  if (!m_file && create_handlers(mem_root))
+    if (!(m_engine_array= (plugin_ref*)
+                  my_malloc(m_tot_parts * sizeof(plugin_ref), MYF(MY_WME))))
+      DBUG_RETURN(TRUE);
+
+    for (i= 0; i < m_tot_parts; i++)
+    {
+      db_type= (enum legacy_db_type) m_file_buffer[PAR_FILE_DB_TYPE_OFFSET + i];
+      if (db_type != first_db_type)
+      {
+        /* We have never allowed different engines */
+        DBUG_ASSERT(0);
+        DBUG_RETURN(TRUE);
+      }
+      m_engine_array[i]= ha_lock_engine(NULL, first_engine);
+    }
+  }
+      
+  if (create_handlers(mem_root))
   {
     clear_handler_file();
     DBUG_RETURN(TRUE);
   }
   DBUG_RETURN(FALSE);
-
-err3:
-  my_afree((gptr) engine_array);
-err2:
-  my_free(file_buffer);
-err1:
-  (void) mysql_file_close(file, MYF(0));
-  DBUG_RETURN(TRUE);
 }
 
 
@@ -2533,13 +2611,14 @@ void ha_data_partition_destroy(HA_DATA_P
 
 int ha_partition::open(const char *name, int mode, uint test_if_locked)
 {
-  char *name_buffer_ptr= m_name_buffer_ptr;
+  char *name_buffer_ptr;
   int error;
   uint alloc_len;
   handler **file;
   char name_buff[FN_REFLEN];
   bool is_not_tmp_table= (table_share->tmp_table == NO_TMP_TABLE);
   ulonglong check_table_flags= 0;
+  MEM_ROOT *mem_root;
   DBUG_ENTER("ha_partition::open");
 
   DBUG_ASSERT(table->s == table_share);
@@ -2547,8 +2626,15 @@ int ha_partition::open(const char *name,
   m_mode= mode;
   m_open_test_lock= test_if_locked;
   m_part_field_array= m_part_info->full_part_field_array;
-  if (get_from_handler_file(name, &table->mem_root))
+  if (clone_mem_root)
+    mem_root= clone_mem_root;
+  else
+    mem_root= &table->mem_root;
+  /* Create handlers and/or partition names if not done before */
+  if (get_from_handler_file(name, mem_root))
     DBUG_RETURN(1);
+  DBUG_ASSERT(m_file && m_name_buffer_ptr);
+  name_buffer_ptr= m_name_buffer_ptr;
   m_start_key.length= 0;
   m_rec0= table->record[0];
   m_rec_length= table_share->reclength;
@@ -2584,7 +2670,7 @@ int ha_partition::open(const char *name,
     DBUG_RETURN(1);
   bitmap_clear_all(&m_bulk_insert_started);
   /* Initialize the bitmap we use to determine what partitions are used */
-  if (!is_clone)
+  if (!is_clone_of)
   {
     if (bitmap_init(&(m_part_info->used_partitions), NULL, m_tot_parts, TRUE))
     {
@@ -2691,7 +2777,7 @@ err_handler:
   while (file-- != m_file)
     (*file)->close();
   bitmap_free(&m_bulk_insert_started);
-  if (!is_clone)
+  if (!is_clone_of)
     bitmap_free(&(m_part_info->used_partitions));
 
   DBUG_RETURN(error);
@@ -2699,16 +2785,26 @@ err_handler:
 
 handler *ha_partition::clone(MEM_ROOT *mem_root)
 {
-  handler *new_handler= get_new_handler(table->s, mem_root,
-                                        table->s->db_type());
-  ((ha_partition*)new_handler)->m_part_info= m_part_info;
-  ((ha_partition*)new_handler)->is_clone= TRUE;
-  if (new_handler && !new_handler->ha_open(table,
-                                           table->s->normalized_path.str,
-                                           table->db_stat,
-                                           HA_OPEN_IGNORE_IF_LOCKED))
-    return new_handler;
-  return NULL;
+  ha_partition *file;
+
+  DBUG_ENTER("ha_partition::clone");
+
+  file= new (mem_root) ha_partition(table->s->db_type(), table->s,
+                                    m_part_info, this, mem_root);
+  if (!file)
+    DBUG_RETURN(NULL);
+
+  if (file->initialize_partition(mem_root))
+    goto error;
+
+  if (file->ha_open(table, table->s->normalized_path.str, table->db_stat,
+                    HA_OPEN_IGNORE_IF_LOCKED))
+    goto error;
+
+  DBUG_RETURN(file);
+error:
+  delete file;
+  DBUG_RETURN(NULL);
 }
 
 
@@ -2739,7 +2835,7 @@ int ha_partition::close(void)
   DBUG_ASSERT(table->s == table_share);
   delete_queue(&m_queue);
   bitmap_free(&m_bulk_insert_started);
-  if (!is_clone)
+  if (!is_clone_of)
     bitmap_free(&(m_part_info->used_partitions));
   file= m_file;
 

=== modified file 'sql/ha_partition.h'
--- a/sql/ha_partition.h	2010-12-03 09:33:29 +0000
+++ b/sql/ha_partition.h	2011-01-24 15:47:28 +0000
@@ -31,6 +31,7 @@ enum partition_keywords
 
 
 #define PARTITION_BYTES_IN_POS 2
+#define PAR_FILE_DB_TYPE_OFFSET 12
 #define PARTITION_ENABLED_TABLE_FLAGS (HA_FILE_BASED | HA_REC_NOT_IN_SEQ)
 #define PARTITION_DISABLED_TABLE_FLAGS (HA_CAN_GEOMETRY | \
                                         HA_CAN_FULLTEXT | \
@@ -149,10 +150,10 @@ private:
   THR_LOCK_DATA lock;                   /* MySQL lock */
 
   /* 
-    TRUE <=> this object was created with ha_partition::clone and doesn't
-    "own" the m_part_info structure.
+    if handler::is_clone_of != NULL this object was created with
+    ha_partition::clone and doesn't "own" the m_part_info structure.
   */
-  bool is_clone;
+
   bool auto_increment_lock;             /**< lock reading/updating auto_inc */
   /**
     Flag to keep the auto_increment lock through out the statement.
@@ -171,6 +172,12 @@ public:
      m_part_info= part_info;
      m_is_sub_partitioned= part_info->is_sub_partitioned();
   }
+  handler *get_handler_for_partition(uint index)
+  {
+    if (!m_file || index >= m_tot_parts)
+      return NULL;
+    return m_file[index];
+  }
   /*
     -------------------------------------------------------------------------
     MODULE create/delete handler object
@@ -183,6 +190,9 @@ public:
     -------------------------------------------------------------------------
   */
     ha_partition(handlerton *hton, TABLE_SHARE * table);
+    ha_partition(handlerton *hton, TABLE_SHARE * table,
+                 partition_info *part_info_arg,
+                 handler *clone_arg, MEM_ROOT *clone_mem_root_arg);
     ha_partition(handlerton *hton, partition_info * part_info);
    ~ha_partition();
   /*
@@ -254,6 +264,7 @@ private:
     And one method to read it in.
   */
   bool create_handler_file(const char *name);
+  bool read_handler_file(const char *name);
   bool get_from_handler_file(const char *name, MEM_ROOT *mem_root);
   bool new_handlers_from_part_info(MEM_ROOT *mem_root);
   bool create_handlers(MEM_ROOT *mem_root);

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2011-01-11 11:33:28 +0000
+++ b/sql/handler.cc	2011-01-24 15:47:28 +0000
@@ -2076,22 +2076,33 @@ int ha_delete_table(THD *thd, handlerton
 /****************************************************************************
 ** General handler functions
 ****************************************************************************/
+
+/**
+  Clone the handler
+    @param mem_root      MEM_ROOT to allocate memory on.
+
+  @return Clone of handler or NULL
+
+  @note The original handler must be opened and locked.
+*/
+
 handler *handler::clone(MEM_ROOT *mem_root)
 {
-  handler *new_handler= get_new_handler(table->s, mem_root, table->s->db_type());
-  /*
-    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)))
+  handler *new_handler;
+  
+  new_handler= get_new_handler(table->s, mem_root, table->s->db_type());
+  if (!new_handler)
+    return NULL;
+  new_handler->is_clone_of= this;
+  new_handler->clone_mem_root= mem_root;
+  if (new_handler->ha_open(table, table->s->normalized_path.str,
+                           table->db_stat,
+                           HA_OPEN_IGNORE_IF_LOCKED))
+  {
+    delete new_handler;
     return NULL;
-  if (new_handler && !new_handler->ha_open(table,
-                                           table->s->normalized_path.str,
-                                           table->db_stat,
-                                           HA_OPEN_IGNORE_IF_LOCKED))
-    return new_handler;
-  return NULL;
+  }
+  return new_handler;
 }
 
 
@@ -2154,13 +2165,18 @@ int handler::ha_open(TABLE *table_arg, c
   }
   else
   {
+    MEM_ROOT *mem_root;
     if (table->s->db_options_in_use & HA_OPTION_READ_ONLY_DATA)
       table->db_stat|=HA_READ_ONLY;
     (void) extra(HA_EXTRA_NO_READCHECK);	// Not needed in SQL
 
-    /* ref is already allocated for us if we're called from handler::clone() */
-    if (!ref && !(ref= (uchar*) alloc_root(&table->mem_root, 
-                                          ALIGN_SIZE(ref_length)*2)))
+    /* Use the mem_root given in ::clone() or the tables mem_root */
+    if (clone_mem_root)
+      mem_root= clone_mem_root;
+    else
+      mem_root= &table->mem_root;
+    if (!ref &&
+        !(ref= (uchar*) alloc_root(mem_root, ALIGN_SIZE(ref_length)*2)))
     {
       close();
       error=HA_ERR_OUT_OF_MEM;

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2010-11-05 11:01:10 +0000
+++ b/sql/handler.h	2011-01-24 15:47:28 +0000
@@ -1237,6 +1237,9 @@ public:
   */
   PSI_table *m_psi;
 
+  handler *is_clone_of;
+  MEM_ROOT *clone_mem_root;
+
   handler(handlerton *ht_arg, TABLE_SHARE *share_arg)
     :table_share(share_arg), table(0),
     estimation_rows_to_insert(0), ht(ht_arg),
@@ -1246,7 +1249,7 @@ public:
     locked(FALSE), implicit_emptied(0),
     pushed_cond(0), next_insert_id(0), insert_id_for_cur_row(0),
     auto_inc_intervals_count(0),
-    m_psi(NULL)
+    m_psi(NULL), is_clone_of(NULL), clone_mem_root(NULL)
     {}
   virtual ~handler(void)
   {

=== modified file 'storage/heap/ha_heap.cc'
--- a/storage/heap/ha_heap.cc	2010-10-06 14:34:28 +0000
+++ b/storage/heap/ha_heap.cc	2011-01-24 15:47:28 +0000
@@ -101,6 +101,16 @@ const char **ha_heap::bas_ext() const
 int ha_heap::open(const char *name, int mode, uint test_if_locked)
 {
   internal_table= test(test_if_locked & HA_OPEN_INTERNAL_TABLE);
+
+  /*
+    Use the original file->s->name instead of table->s->path if cloned.
+    This is needed by Windows where the clone() call sees
+    '/'-delimited path in table->s->path, while ha_heap::open() was called 
+    with '\'-delimited path.
+  */
+  if (is_clone_of)
+    name= ((ha_heap*)is_clone_of)->hp_info()->s->name;
+
   if (internal_table || (!(file= heap_open(name, mode)) && my_errno == ENOENT))
   {
     HP_CREATE_INFO create_info;
@@ -152,26 +162,6 @@ int ha_heap::close(void)
 
 
 /*
-  Create a copy of this table
-
-  DESCRIPTION
-    Do same as default implementation but use file->s->name instead of 
-    table->s->path. This is needed by Windows where the clone() call sees
-    '/'-delimited path in table->s->path, while ha_peap::open() was called 
-    with '\'-delimited path.
-*/
-
-handler *ha_heap::clone(MEM_ROOT *mem_root)
-{
-  handler *new_handler= get_new_handler(table->s, mem_root, table->s->db_type());
-  if (new_handler && !new_handler->ha_open(table, file->s->name, table->db_stat,
-                                           HA_OPEN_IGNORE_IF_LOCKED))
-    return new_handler;
-  return NULL;  /* purecov: inspected */
-}
-
-
-/*
   Compute which keys to use for scanning
 
   SYNOPSIS

=== modified file 'storage/heap/ha_heap.h'
--- a/storage/heap/ha_heap.h	2010-10-06 14:34:28 +0000
+++ b/storage/heap/ha_heap.h	2011-01-24 15:47:28 +0000
@@ -35,7 +35,6 @@ class ha_heap: public handler
 public:
   ha_heap(handlerton *hton, TABLE_SHARE *table);
   ~ha_heap() {}
-  handler *clone(MEM_ROOT *mem_root);
   const char *table_type() const
   {
     return (table->in_use->variables.sql_mode & MODE_MYSQL323) ?
@@ -118,6 +117,7 @@ public:
     return memcmp(ref1, ref2, sizeof(HEAP_PTR));
   }
   bool check_if_incompatible_data(HA_CREATE_INFO *info, uint table_changes);
+  HP_INFO *hp_info() { return file; }
 private:
   void update_key_stats();
 };

=== modified file 'storage/myisam/ha_myisam.cc'
--- a/storage/myisam/ha_myisam.cc	2011-01-11 09:07:37 +0000
+++ b/storage/myisam/ha_myisam.cc	2011-01-24 15:47:28 +0000
@@ -643,14 +643,6 @@ ha_myisam::ha_myisam(handlerton *hton, T
    can_enable_indexes(1)
 {}
 
-handler *ha_myisam::clone(MEM_ROOT *mem_root)
-{
-  ha_myisam *new_handler= static_cast <ha_myisam *>(handler::clone(mem_root));
-  if (new_handler)
-    new_handler->file->state= file->state;
-  return new_handler;
-}
-
 
 static const char *ha_myisam_exts[] = {
   ".MYI",
@@ -746,6 +738,8 @@ int ha_myisam::open(const char *name, in
     table->key_info[i].block_size= file->s->keyinfo[i].block_length;
   }
   my_errno= 0;
+  if (is_clone_of)
+    file->state= ((ha_myisam*)is_clone_of)->file_ptr()->state;
   goto end;
  err:
   this->close();

=== modified file 'storage/myisam/ha_myisam.h'
--- a/storage/myisam/ha_myisam.h	2010-10-06 14:34:28 +0000
+++ b/storage/myisam/ha_myisam.h	2011-01-24 15:47:28 +0000
@@ -50,7 +50,6 @@ class ha_myisam: public handler
  public:
   ha_myisam(handlerton *hton, TABLE_SHARE *table_arg);
   ~ha_myisam() {}
-  handler *clone(MEM_ROOT *mem_root);
   const char *table_type() const { return "MyISAM"; }
   const char *index_type(uint key_number);
   const char **bas_ext() const;

=== modified file 'storage/myisammrg/ha_myisammrg.cc'
--- a/storage/myisammrg/ha_myisammrg.cc	2010-10-20 19:02:59 +0000
+++ b/storage/myisammrg/ha_myisammrg.cc	2011-01-24 15:47:28 +0000
@@ -118,7 +118,7 @@ static handler *myisammrg_create_handler
 */
 
 ha_myisammrg::ha_myisammrg(handlerton *hton, TABLE_SHARE *table_arg)
-  :handler(hton, table_arg), file(0), is_cloned(0)
+  :handler(hton, table_arg), file(0)
 {
   init_sql_alloc(&children_mem_root,
                  FN_REFLEN + ALLOC_ROOT_MIN_BLOCK_SIZE, 0);
@@ -371,8 +371,10 @@ int ha_myisammrg::open(const char *name,
   my_errno= 0;
 
   /* retrieve children table list. */
-  if (is_cloned)
+  if (is_clone_of)
   {
+    MYRG_TABLE    *u_table,*newu_table;
+    MYRG_INFO     *clone_file= ((ha_myisammrg*)is_clone_of)->myrg_info();
     /*
       Open and attaches the MyISAM tables,that are under the MERGE table 
       parent, on the MyISAM storage engine interface directly within the
@@ -390,6 +392,20 @@ int ha_myisammrg::open(const char *name,
     file->children_attached= TRUE;
 
     info(HA_STATUS_NO_LOCK | HA_STATUS_VARIABLE | HA_STATUS_CONST);
+    /*
+      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= file->open_tables;
+    for (u_table= clone_file->open_tables;
+         u_table < clone_file->end_table;
+         u_table++)
+    {
+      newu_table->table->state= u_table->table->state;
+      newu_table++;
+    }
   }
   else if (!(file= myrg_parent_open(name, myisammrg_parent_open_callback, this)))
   {
@@ -677,56 +693,6 @@ CPP_UNNAMED_NS_END
 
 
 /**
-   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;
-  }
-
-  if (new_handler->ha_open(table, 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++;
-  }
-
-  return new_handler;
- }
-
-
-/**
   Attach children to a MERGE table.
 
   @return status
@@ -1033,7 +999,7 @@ int ha_myisammrg::close(void)
     There are cases where children are not explicitly detached before
     close. detach_children() protects itself against double detach.
   */
-  if (!is_cloned)
+  if (!is_clone_of)
     detach_children();
 
   rc= myrg_close(file);
@@ -1394,7 +1360,7 @@ int ha_myisammrg::external_lock(THD *thd
     If this handler instance has been cloned, we still must call
     myrg_lock_database().
   */
-  if (is_cloned)
+  if (is_clone_of)
     return myrg_lock_database(file, lock_type);
   return 0;
 }

=== modified file 'storage/myisammrg/ha_myisammrg.h'
--- a/storage/myisammrg/ha_myisammrg.h	2010-10-06 14:34:28 +0000
+++ b/storage/myisammrg/ha_myisammrg.h	2011-01-24 15:47:28 +0000
@@ -70,7 +70,6 @@ public:
 class ha_myisammrg: public handler
 {
   MYRG_INFO *file;
-  my_bool is_cloned;                    /* This instance has been cloned */
 
 public:
   MEM_ROOT      children_mem_root;      /* mem root for children list */
@@ -110,7 +109,6 @@ public:
   int add_children_list(void);
   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);


Attachment: [text/bzr-bundle] bzr/mattias.jonsson@oracle.com-20110124154728-722elikkya90e1xm.bundle
Thread
bzr commit into mysql-5.5 branch (mattias.jonsson:3271) Bug#59316Mattias Jonsson24 Jan
  • Re: bzr commit into mysql-5.5 branch (mattias.jonsson:3271)Bug#59316Sergey Vojtovich3 Feb