List:Commits« Previous MessageNext Message »
From:Luis Soares Date:January 26 2011 4:00pm
Subject:bzr commit into mysql-trunk-w5597 branch (luis.soares:3209) WL#5597
View as plain text  
#At file:///home/lsoares/Workspace/bzr/work/features/wl5597/mysql-trunk-wl5597/ based on revid:luis.soares@stripped

 3209 Luis Soares	2011-01-26
      WL#5597
      
      Addressing Alfranio's review comments.

    modified:
      mysql-test/suite/rpl/t/rpl_row_hash_scan.test
      mysql-test/suite/sys_vars/r/slave_rows_search_algorithms_basic.result
      mysql-test/suite/sys_vars/t/slave_rows_search_algorithms_basic.test
      sql/log_event.cc
      sql/log_event.h
      sql/rpl_utility.cc
      sql/rpl_utility.h
      sql/sys_vars.cc
=== modified file 'mysql-test/suite/rpl/t/rpl_row_hash_scan.test'
--- a/mysql-test/suite/rpl/t/rpl_row_hash_scan.test	2010-11-23 00:08:01 +0000
+++ b/mysql-test/suite/rpl/t/rpl_row_hash_scan.test	2011-01-26 16:00:19 +0000
@@ -215,4 +215,4 @@ DELETE FROM t1;
 
 -- source include/master-slave-reset.inc
 
-SET @@global.slave_rows_search_algorithms= @saved_slave_rows_search_algorithms;
\ No newline at end of file
+SET @@global.slave_rows_search_algorithms= @saved_slave_rows_search_algorithms;

=== modified file 'mysql-test/suite/sys_vars/r/slave_rows_search_algorithms_basic.result'
--- a/mysql-test/suite/sys_vars/r/slave_rows_search_algorithms_basic.result	2011-01-11 01:03:43 +0000
+++ b/mysql-test/suite/sys_vars/r/slave_rows_search_algorithms_basic.result	2011-01-26 16:00:19 +0000
@@ -11,10 +11,9 @@ SELECT @@global.slave_rows_search_algori
 @@global.slave_rows_search_algorithms
 HASH_SCAN
 SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS='INDEX_SCAN';
-ERROR 42000: Variable 'slave_rows_search_algorithms' can't be set to the value of 'INDEX_SCAN'
 SELECT @@global.slave_rows_search_algorithms;
 @@global.slave_rows_search_algorithms
-HASH_SCAN
+INDEX_SCAN
 SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS='TABLE_SCAN,HASH_SCAN';
 SELECT @@global.slave_rows_search_algorithms;
 @@global.slave_rows_search_algorithms
@@ -46,6 +45,15 @@ ERROR 42000: Variable 'slave_rows_search
 SELECT @@global.slave_rows_search_algorithms;
 @@global.slave_rows_search_algorithms
 INDEX_SCAN,HASH_SCAN
+SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS=NULL;
+ERROR 42000: Variable 'slave_rows_search_algorithms' can't be set to the value of 'NULL'
+SELECT @@global.slave_rows_search_algorithms;
+@@global.slave_rows_search_algorithms
+INDEX_SCAN,HASH_SCAN
+SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS=DEFAULT;
+SELECT @@global.slave_rows_search_algorithms;
+@@global.slave_rows_search_algorithms
+TABLE_SCAN,INDEX_SCAN
 set global slave_rows_search_algorithms = @saved_slave_rows_search_algorithms;
 SELECT @@global.slave_rows_search_algorithms;
 @@global.slave_rows_search_algorithms

=== modified file 'mysql-test/suite/sys_vars/t/slave_rows_search_algorithms_basic.test'
--- a/mysql-test/suite/sys_vars/t/slave_rows_search_algorithms_basic.test	2011-01-11 01:03:43 +0000
+++ b/mysql-test/suite/sys_vars/t/slave_rows_search_algorithms_basic.test	2011-01-26 16:00:19 +0000
@@ -11,7 +11,6 @@ SELECT @@global.slave_rows_search_algori
 SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS='HASH_SCAN';
 SELECT @@global.slave_rows_search_algorithms;
 
--- error ER_WRONG_VALUE_FOR_VAR
 SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS='INDEX_SCAN';
 SELECT @@global.slave_rows_search_algorithms;
 
@@ -41,5 +40,12 @@ SELECT @@global.slave_rows_search_algori
 SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS='1';
 SELECT @@global.slave_rows_search_algorithms;
 
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS=NULL;
+SELECT @@global.slave_rows_search_algorithms;
+
+SET GLOBAL SLAVE_ROWS_SEARCH_ALGORITHMS=DEFAULT;
+SELECT @@global.slave_rows_search_algorithms;
+
 set global slave_rows_search_algorithms = @saved_slave_rows_search_algorithms;
 SELECT @@global.slave_rows_search_algorithms;

=== modified file 'sql/log_event.cc'
--- a/sql/log_event.cc	2011-01-11 00:59:45 +0000
+++ b/sql/log_event.cc	2011-01-26 16:00:19 +0000
@@ -7776,8 +7776,7 @@ static uint decide_row_lookup_algorithm(
   else
   {
     /**
-       Blackhole does not use hash scan.  (NOTE: This is a hackish
-       implementation and I know it).
+       Blackhole does not use hash scan.
 
        TODO: remove this DB_TYPE_BLACKHOLE_DB dependency.
     */
@@ -8002,10 +8001,9 @@ int Rows_log_event::do_index_scan_and_up
   DBUG_ENTER("Rows_log_event::do_index_scan_and_update");
   DBUG_ASSERT(m_table && m_table->in_use != NULL);
 
+  KEY *keyinfo= NULL;
   TABLE *table= m_table;
   int error= 0;
-  KEY *keyinfo;
-  uint key;
   const uchar *saved_m_curr_row= m_curr_row;
 
   /*
@@ -8024,7 +8022,15 @@ int Rows_log_event::do_index_scan_and_up
   // Temporary fix to find out why it fails [/Matz]
   memcpy(m_table->read_set->bitmap, m_cols.bitmap, (m_table->read_set->n_bits + 7) / 8);
 
-  if (!is_any_column_signaled_for_table(table, &m_cols))
+  /* 
+    Trying to do an index scan without a usable key 
+    This is a valid state because we allow the user
+    to set Slave_rows_search_algorithm= 'INDEX_SCAN'.
+
+    Therefore on tables with no indexes we will end
+    up here.
+   */
+  if (m_key_index >= MAX_KEY)
   {
     error= HA_ERR_END_OF_FILE;
     goto end;
@@ -8035,9 +8041,11 @@ int Rows_log_event::do_index_scan_and_up
   DBUG_DUMP("record[0]", table->record[0], table->s->reclength);
 #endif
 
-  if ((key= search_key_in_table(table, &m_cols, PRI_KEY_FLAG)) >= MAX_KEY)
+  if (m_key_index != m_table->s->primary_key)
+  {
     /* we dont have a PK, or PK is not usable with BI values */
     goto INDEX_SCAN;
+  }
 
   if ((table->file->ha_table_flags() & HA_PRIMARY_KEY_REQUIRED_FOR_POSITION))
   {
@@ -8084,149 +8092,137 @@ int Rows_log_event::do_index_scan_and_up
 
 INDEX_SCAN:
 
+  /* we will be using the KEY in m_key_index */
+  keyinfo=table->key_info + m_key_index;
+
+  /* Fill key data for the row */
+  DBUG_ASSERT(m_key);
+  key_copy(m_key, table->record[0], keyinfo, 0);
+
   /*
     Save copy of the record in table->record[1]. It might be needed
     later if linear search is used to find exact match.
    */
   store_record(table,record[1]);
 
-  if ((key= search_key_in_table(table, &m_cols,
-                                (PRI_KEY_FLAG | UNIQUE_KEY_FLAG | MULTIPLE_KEY_FLAG)))
-       >= MAX_KEY)
-    /* we dont have a key, or no key is suitable for the BI values */
+  DBUG_PRINT("info",("locating record using a key (index_read)"));
+
+  /* The m_key_index'th key is active and usable: search the table using the index */
+  if (!table->file->inited && (error= table->file->ha_index_init(m_key_index, FALSE)))
   {
-    error= HA_ERR_KEY_NOT_FOUND;
+    DBUG_PRINT("info",("ha_index_init returns error %d",error));
     goto end;
   }
 
-  {
-    keyinfo= table->key_info + key;
+  /*
+    Don't print debug messages when running valgrind since they can
+    trigger false warnings.
+   */
+#ifndef HAVE_purify
+  DBUG_DUMP("key data", m_key, keyinfo->key_length);
+#endif
 
+  /*
+    We need to set the null bytes to ensure that the filler bit are
+    all set when returning.  There are storage engines that just set
+    the necessary bits on the bytes and don't set the filler bits
+    correctly.
+  */
+  if (table->s->null_bytes > 0)
+    table->record[0][table->s->null_bytes - 1]|=
+      256U - (1U << table->s->last_null_bit_pos);
 
-    DBUG_PRINT("info",("locating record using primary key (index_read)"));
+  if ((error= table->file->ha_index_read_map(table->record[0], m_key,
+                                             HA_WHOLE_KEY,
+                                             HA_READ_KEY_EXACT)))
+  {
+    DBUG_PRINT("info",("no record matching the key found in the table"));
+    if (error == HA_ERR_RECORD_DELETED)
+      error= HA_ERR_KEY_NOT_FOUND;
+    goto end;
+  }
 
-    /* The key'th key is active and usable: search the table using the index */
-    if (!table->file->inited && (error= table->file->ha_index_init(key, FALSE)))
+  /*
+    Don't print debug messages when running valgrind since they can
+    trigger false warnings.
+   */
+#ifndef HAVE_purify
+  DBUG_PRINT("info",("found first matching record"));
+  DBUG_DUMP("record[0]", table->record[0], table->s->reclength);
+#endif
+  /*
+    Below is a minor "optimization".  If the key (i.e., key number
+    0) has the HA_NOSAME flag set, we know that we have found the
+    correct record (since there can be no duplicates); otherwise, we
+    have to compare the record with the one found to see if it is
+    the correct one.
+
+    CAVEAT! This behaviour is essential for the replication of,
+    e.g., the mysql.proc table since the correct record *shall* be
+    found using the primary key *only*.  There shall be no
+    comparison of non-PK columns to decide if the correct record is
+    found.  I can see no scenario where it would be incorrect to
+    chose the row to change only using a PK or an UNNI.
+  */
+  if (keyinfo->flags & HA_NOSAME || m_key_index == table->s->primary_key)
+  {
+    /* Unique does not have non nullable part */
+    if (!(table->key_info->flags & (HA_NULL_PART_KEY)))      
+      goto end;  // record found
+    else
     {
-      DBUG_PRINT("info",("ha_index_init returns error %d",error));
-      goto end;
-    }
+      KEY *keyinfo= table->key_info;
+      /*
+        Unique has nullable part. We need to check if there is any field in the
+        BI image that is null and part of UNNI.
+      */
+      bool null_found= FALSE;
+      for (uint i=0; i < keyinfo->key_parts && !null_found; i++)
+      {
+        uint fieldnr= keyinfo->key_part[i].fieldnr - 1;
+        Field **f= table->field+fieldnr;
+        null_found= (*f)->is_null();
+      }
 
-    /* Fill key data for the row */
+      if (!null_found)
+        goto end;           // record found
 
-    DBUG_ASSERT(m_key);
-    key_copy(m_key, table->record[0], keyinfo, 0);
+      /* else fall through to index scan */
+    }
+  }
 
-    /*
-      Don't print debug messages when running valgrind since they can
-      trigger false warnings.
-     */
-#ifndef HAVE_purify
-    DBUG_DUMP("key data", m_key, keyinfo->key_length);
-#endif
+  /*
+    In case key is not unique, we still have to iterate over records found
+    and find the one which is identical to the row given. A copy of the
+    record we are looking for is stored in record[1].
+   */
+  DBUG_PRINT("info",("non-unique index, scanning it to find matching record"));
 
+  while (record_compare(table, &m_cols))
+  {
     /*
-      We need to set the null bytes to ensure that the filler bit are
-      all set when returning.  There are storage engines that just set
-      the necessary bits on the bytes and don't set the filler bits
-      correctly.
+      We need to set the null bytes to ensure that the filler bit
+      are all set when returning.  There are storage engines that
+      just set the necessary bits on the bytes and don't set the
+      filler bits correctly.
+
+      TODO[record format ndb]: Remove this code once NDB returns the
+      correct record format.
     */
     if (table->s->null_bytes > 0)
+    {
       table->record[0][table->s->null_bytes - 1]|=
         256U - (1U << table->s->last_null_bit_pos);
+    }
 
-    if ((error= table->file->ha_index_read_map(table->record[0], m_key,
-                                               HA_WHOLE_KEY,
-                                               HA_READ_KEY_EXACT)))
+    while ((error= table->file->ha_index_next(table->record[0])))
     {
-      DBUG_PRINT("info",("no record matching the key found in the table"));
+      /* We just skip records that has already been deleted */
       if (error == HA_ERR_RECORD_DELETED)
-        error= HA_ERR_KEY_NOT_FOUND;
+        continue;
+      DBUG_PRINT("info",("no record matching the given row found"));
       goto end;
     }
-
-    /*
-      Don't print debug messages when running valgrind since they can
-      trigger false warnings.
-     */
-#ifndef HAVE_purify
-    DBUG_PRINT("info",("found first matching record"));
-    DBUG_DUMP("record[0]", table->record[0], table->s->reclength);
-#endif
-    /*
-      Below is a minor "optimization".  If the key (i.e., key number
-      0) has the HA_NOSAME flag set, we know that we have found the
-      correct record (since there can be no duplicates); otherwise, we
-      have to compare the record with the one found to see if it is
-      the correct one.
-
-      CAVEAT! This behaviour is essential for the replication of,
-      e.g., the mysql.proc table since the correct record *shall* be
-      found using the primary key *only*.  There shall be no
-      comparison of non-PK columns to decide if the correct record is
-      found.  I can see no scenario where it would be incorrect to
-      chose the row to change only using a PK or an UNNI.
-    */
-    if (keyinfo->flags & HA_NOSAME || key == table->s->primary_key)
-    {
-      /* Unique does not have non nullable part */
-      if (!(table->key_info->flags & (HA_NULL_PART_KEY)))      
-        goto end;  // record found
-      else
-      {
-        KEY *keyinfo= table->key_info;
-        /*
-          Unique has nullable part. We need to check if there is any field in the
-          BI image that is null and part of UNNI.
-        */
-        bool null_found= FALSE;
-        for (uint i=0; i < keyinfo->key_parts && !null_found; i++)
-        {
-          uint fieldnr= keyinfo->key_part[i].fieldnr - 1;
-          Field **f= table->field+fieldnr;
-          null_found= (*f)->is_null();
-        }
-
-        if (!null_found)
-          goto end;           // record found
-
-        /* else fall through to index scan */
-      }
-    }
-
-    /*
-      In case key is not unique, we still have to iterate over records found
-      and find the one which is identical to the row given. A copy of the
-      record we are looking for is stored in record[1].
-     */
-    DBUG_PRINT("info",("non-unique index, scanning it to find matching record"));
-
-    while (record_compare(table, &m_cols))
-    {
-      /*
-        We need to set the null bytes to ensure that the filler bit
-        are all set when returning.  There are storage engines that
-        just set the necessary bits on the bytes and don't set the
-        filler bits correctly.
-
-        TODO[record format ndb]: Remove this code once NDB returns the
-        correct record format.
-      */
-      if (table->s->null_bytes > 0)
-      {
-        table->record[0][table->s->null_bytes - 1]|=
-          256U - (1U << table->s->last_null_bit_pos);
-      }
-
-      while ((error= table->file->ha_index_next(table->record[0])))
-      {
-        /* We just skip records that has already been deleted */
-        if (error == HA_ERR_RECORD_DELETED)
-          continue;
-        DBUG_PRINT("info",("no record matching the given row found"));
-        goto end;
-      }
-    }
   }
 
 end:
@@ -8378,7 +8374,8 @@ int Rows_log_event::do_hash_scan_and_upd
             m_curr_row_end= entry->bi_ends;
 
             /* we don't need this entry anymore, just delete it */
-            m_hash.del(entry);
+            if ((error= m_hash.del(entry)))
+              goto err;
             
             if ((error= do_apply_row(rli)))
             {
@@ -8414,9 +8411,16 @@ close_table:
                           " (ha_rnd_next returns %d)",error));
     }
     m_table->file->ha_rnd_end();
+  
+    if (error == HA_ERR_RECORD_DELETED)
+      error= 0;
+
+    DBUG_ASSERT((m_hash.is_empty() && !error) || 
+                (!m_hash.is_empty() && error));
   }
 
 err:
+
   if (m_hash.is_empty() && !error)
   {
     /**
@@ -8426,7 +8430,7 @@ err:
     m_curr_row= saved_last_m_curr_row;
     m_curr_row_end= m_rows_end;
   }
-  DBUG_ASSERT((m_hash.is_empty()) ? (error == 0) : (!m_hash.is_empty()));
+
   DBUG_RETURN(error);  
 }
 
@@ -8745,7 +8749,7 @@ int Rows_log_event::do_apply_event(Relay
       }
       else /* WRITE ROWS EVENTS store the bitmap in m_cols instead of m_cols_ai */      
       {
-        DBUG_PRINT_BITSET("debug", "Setting table's write_set from: %s", &m_cols_ai);
+        DBUG_PRINT_BITSET("debug", "Setting table's write_set from: %s", &m_cols);
         bitmap_intersect(table->write_set,&m_cols);
       }
     }
@@ -10072,18 +10076,48 @@ Delete_rows_log_event::Delete_rows_log_e
 int 
 Delete_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const)
 {
-  if (m_table->s->keys > 0)
+  /* 
+     Prepare memory structures for search operations. If
+     search is performed:
+    
+     1. using hash search => initialize the hash
+     2. using key => decide on key to use and allocate mem structures
+     3. using table scan => do nothing
+   */
+  m_rows_lookup_algorithm= decide_row_lookup_algorithm(m_table, &m_cols, get_type_code());
+
+  if (m_rows_lookup_algorithm == ROW_LOOKUP_HASH_SCAN)
   {
-    // Allocate buffer for key searches
-    m_key= (uchar*)my_malloc(MAX_KEY_LENGTH, MYF(MY_WME));
-    if (!m_key)
+    if (m_hash.init())
       return HA_ERR_OUT_OF_MEM;
   }
+  /* check if we have a suitable key and if so allocate space */
+  else if ((m_table->s->keys > 0) && 
+           (m_rows_lookup_algorithm == ROW_LOOKUP_INDEX_SCAN))
+  {
+    m_key_index= MAX_KEY;
+    m_key_index= search_key_in_table(m_table, &m_cols, 
+                                     (PRI_KEY_FLAG |        // primary
+                                      UNIQUE_KEY_FLAG |     // unique
+                                      MULTIPLE_KEY_FLAG));  // regular
 
-  /* will we be using a hash to lookup rows? If so, initialize it. */
-  m_rows_lookup_algorithm= decide_row_lookup_algorithm(m_table, &m_cols, get_type_code());
-  if (m_rows_lookup_algorithm == ROW_LOOKUP_HASH_SCAN)
-    m_hash.init();
+    /* 
+      since we have a suitable key, lets allocate space 
+      for storing it later 
+     */
+    if (m_key_index < MAX_KEY)
+    {
+      // Allocate buffer for key searches
+      m_key= (uchar*)my_malloc(MAX_KEY_LENGTH, MYF(MY_WME));
+      if (!m_key)
+        return HA_ERR_OUT_OF_MEM;
+    }
+
+    /* 
+       Do not report error here if no suitable index is found.
+       We will be doing it on the search routine.
+    */
+  }
 
   return 0;
 }
@@ -10094,8 +10128,13 @@ Delete_rows_log_event::do_after_row_oper
 {
   /*error= ToDo:find out what this should really be, this triggers close_scan in nbd, returning error?*/
   m_table->file->ha_index_or_rnd_end();
-  my_free(m_key);
-  m_key= NULL;
+  if ((m_rows_lookup_algorithm == ROW_LOOKUP_INDEX_SCAN) &&
+      (m_key_index < MAX_KEY))
+  {
+    my_free(m_key); // Free for multi_malloc
+    m_key= NULL; 
+    m_key_index= MAX_KEY;
+  }
 
   /* we don't need the hash anymore, free it */
   if (m_rows_lookup_algorithm == ROW_LOOKUP_HASH_SCAN)
@@ -10190,20 +10229,51 @@ Update_rows_log_event::Update_rows_log_e
 int 
 Update_rows_log_event::do_before_row_operations(const Slave_reporting_capability *const)
 {
-  if (m_table->s->keys > 0)
+  /* 
+     Prepare memory structures for search operations. If
+     search is performed:
+    
+     1. using hash search => initialize the hash
+     2. using key => decide on key to use and allocate mem structures
+     3. using table scan => do nothing
+   */
+  m_rows_lookup_algorithm= decide_row_lookup_algorithm(m_table, &m_cols, get_type_code());
+
+  if (m_rows_lookup_algorithm == ROW_LOOKUP_HASH_SCAN)
   {
-    // Allocate buffer for key searches
-    m_key= (uchar*)my_malloc(m_table->key_info->key_length, MYF(MY_WME));
-    if (!m_key)
+    if (m_hash.init())
       return HA_ERR_OUT_OF_MEM;
   }
+  /* check if we have a suitable key and if so allocate space */
+  else if ((m_table->s->keys > 0) && 
+           (m_rows_lookup_algorithm == ROW_LOOKUP_INDEX_SCAN))
+  {
+    m_key_index= MAX_KEY;
+    m_key_index= search_key_in_table(m_table, &m_cols, 
+                                     (PRI_KEY_FLAG |        // primary
+                                      UNIQUE_KEY_FLAG |     // unique
+                                      MULTIPLE_KEY_FLAG));  // regular
+
+    /* 
+      since we have a suitable key, lets allocate space 
+      for storing it later 
+     */
+    if (m_key_index < MAX_KEY)
+    {
+      // Allocate buffer for key searches
+      m_key= (uchar*)my_malloc(MAX_KEY_LENGTH, MYF(MY_WME));
+      if (!m_key)
+        return HA_ERR_OUT_OF_MEM;
+    }
+
+    /* 
+       Do not report error here if no suitable index is found.
+       We will be doing it on the search routine.
+    */
+  }
 
   m_table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
 
-  /* will we be using a hash to lookup rows? If so, initialize it. */
-  m_rows_lookup_algorithm= decide_row_lookup_algorithm(m_table, &m_cols, get_type_code());
-  if (m_rows_lookup_algorithm == ROW_LOOKUP_HASH_SCAN)
-    m_hash.init();
   return 0;
 }
 
@@ -10213,8 +10283,14 @@ Update_rows_log_event::do_after_row_oper
 {
   /*error= ToDo:find out what this should really be, this triggers close_scan in nbd, returning error?*/
   m_table->file->ha_index_or_rnd_end();
-  my_free(m_key); // Free for multi_malloc
-  m_key= NULL;
+
+  if ((m_table->s->keys > 0) &&
+      (m_rows_lookup_algorithm == ROW_LOOKUP_INDEX_SCAN))
+  {
+    my_free(m_key); // Free for multi_malloc
+    m_key= NULL;
+    m_key_index= MAX_KEY;
+  }
 
   /* we don't need the hash anymore, free it */
   if (m_rows_lookup_algorithm == ROW_LOOKUP_HASH_SCAN)  

=== modified file 'sql/log_event.h'
--- a/sql/log_event.h	2011-01-11 00:59:45 +0000
+++ b/sql/log_event.h	2011-01-26 16:00:19 +0000
@@ -3752,6 +3752,7 @@ protected:
   const uchar *m_curr_row;     /* Start of the row being processed */
   const uchar *m_curr_row_end; /* One-after the end of the current row */
   uchar    *m_key;      /* Buffer to keep key value during searches */
+  uint     m_key_index;
 
   int find_row(const Relay_log_info *const);
 

=== modified file 'sql/rpl_utility.cc'
--- a/sql/rpl_utility.cc	2011-01-11 00:59:45 +0000
+++ b/sql/rpl_utility.cc	2011-01-26 16:00:19 +0000
@@ -1058,6 +1058,8 @@ table_def::~table_def()
 
 #ifndef MYSQL_CLIENT
 
+#define HASH_ROWS_POS_SEARCH_INVALID -1
+
 /**
   Utility methods for handling row based operations.
  */ 
@@ -1071,9 +1073,8 @@ table_def::~table_def()
    entry_ptr= preamble_ptr+1;
    
    preamble_ptr  -----> |-HASH_ROWS_POS_PREAMBLE--|
-                        | - key                   |
-                        | - length                |
                         | - hash_value            |
+                        | - length                |
                         | - is_search_state_inited|
                         | - search_state          |
                         |                         |
@@ -1098,21 +1099,16 @@ typedef struct hash_row_pos_preamble
 {
   
   /**
-     The pointer to the hash table key.
+     The actual key.
   */
-  uchar* key;
-  
+  my_hash_value_type hash_value;
+
   /**
      Length of the key.
   */
   uint length;
   
   /**
-     The actual key.
-  */
-  my_hash_value_type hash_value;
-  
-  /**
      The search state used to iterate over multiple entries for a
      given key.
   */
@@ -1163,7 +1159,7 @@ hash_slave_rows_get_key(const uchar *rec
   HASH_ROW_POS_PREAMBLE *preamble=(HASH_ROW_POS_PREAMBLE *) record;
   *length= preamble->length;
 
-  DBUG_RETURN((uchar*) preamble->key);
+  DBUG_RETURN((uchar*) &preamble->hash_value);
 }
 
 static void 
@@ -1186,16 +1182,16 @@ bool Hash_slave_rows::is_empty(void)
 
 bool Hash_slave_rows::init(void)
 {
-  my_hash_init(&m_hash,
-               &my_charset_bin,                /* the charater set information */
-               16 /* TODO */,                  /* growth size */
-               0,                              /* key offset */
-               0,                              /* key length */
-               hash_slave_rows_get_key,                        /* get function pointer */
-               (my_hash_free_key) hash_slave_rows_free_entry,  /* freefunction pointer */
-               MYF(0));                        /* flags */
-
-  return 0;
+  if (my_hash_init(&m_hash,
+                   &my_charset_bin,                /* the charater set information */
+                   16 /* TODO */,                  /* growth size */
+                   0,                              /* key offset */
+                   0,                              /* key length */
+                   hash_slave_rows_get_key,                        /* get function pointer */
+                   (my_hash_free_key) hash_slave_rows_free_entry,  /* freefunction pointer */
+                   MYF(0)))                        /* flags */
+    return true;
+  return false;
 }
 
 bool Hash_slave_rows::deinit(void)
@@ -1230,10 +1226,9 @@ HASH_ROW_POS_ENTRY* Hash_slave_rows::mak
   /**
      Filling in the preamble.
    */
-  preamble->key= (uchar*)&preamble->hash_value;
+  preamble->hash_value= 0;
   preamble->length= sizeof(my_hash_value_type);
-  preamble->search_state= -1;
-  preamble->hash_value= -1;
+  preamble->search_state= HASH_ROWS_POS_SEARCH_INVALID;
   preamble->is_search_state_inited= false;
     
   /**
@@ -1311,6 +1306,7 @@ Hash_slave_rows::get(TABLE *table, MY_BI
 bool Hash_slave_rows::next(HASH_ROW_POS_ENTRY** entry)
 {
   DBUG_ENTER("Hash_slave_rows::next");
+  DBUG_ASSERT(*entry);
 
   if (*entry == NULL)
     DBUG_RETURN(true);
@@ -1328,7 +1324,7 @@ bool Hash_slave_rows::next(HASH_ROW_POS_
     used in the search below (and search state is used in a
     one-time-only basis).
    */
-  preamble->search_state= -1;
+  preamble->search_state= HASH_ROWS_POS_SEARCH_INVALID;
   preamble->is_search_state_inited= false;
   
   DBUG_PRINT("debug", ("Looking for record with key=%u in the hash (next).", key));
@@ -1361,9 +1357,9 @@ bool
 Hash_slave_rows::del(HASH_ROW_POS_ENTRY* entry)
 {
   DBUG_ENTER("Hash_slave_rows::del");
-  if (entry)
-    my_hash_delete(&m_hash, (uchar *) get_preamble(entry));
-  else
+  DBUG_ASSERT(entry);
+
+  if (my_hash_delete(&m_hash, (uchar *) get_preamble(entry)))
     DBUG_RETURN(true);
   DBUG_RETURN(false);
 }

=== modified file 'sql/rpl_utility.h'
--- a/sql/rpl_utility.h	2011-01-11 00:59:45 +0000
+++ b/sql/rpl_utility.h	2011-01-26 16:00:19 +0000
@@ -59,8 +59,8 @@ class Hash_slave_rows
 public:
 
   /**
-     This member function allocates an entry to be added to the hash
-     table. It should be called before calling member function add.
+     Allocates an entry to be added to the hash table. It should be
+     called before calling member function add.
      
      @param bi_start the position to where in the rows buffer the
                      before image begins.
@@ -76,7 +76,7 @@ public:
                                  const uchar *ai_start, const uchar *ai_ends);
 
   /**
-     Member function that puts data into the hash table.
+     Puts data into the hash table.
 
      @param table   The table holding the buffer used to calculate the
                     key, ie, table->record[0].
@@ -88,8 +88,8 @@ public:
   bool put(TABLE* table, MY_BITMAP *cols, HASH_ROW_POS_ENTRY* entry);
 
   /**
-     This member function gets the entry, from the hash table, that
-     matches the data in table->record[0] and signaled using cols.
+     Gets the entry, from the hash table, that matches the data in
+     table->record[0] and signaled using cols.
      
      @param table   The table holding the buffer containing data used to
                     make the entry lookup.
@@ -103,11 +103,11 @@ public:
   HASH_ROW_POS_ENTRY* get(TABLE *table, MY_BITMAP *cols);
 
   /**
-     This member function gets the entry that stands next to the one
-     pointed to by *entry. Before calling this member function, the
-     entry that one uses as parameter must have: 1. been obtained
-     through get() or next() invocations; and 2. must have not been
-     used before in a next() operation.
+     Gets the entry that stands next to the one pointed to by
+     *entry. Before calling this member function, the entry that one
+     uses as parameter must have: 1. been obtained through get() or
+     next() invocations; and 2. must have not been used before in a
+     next() operation.
 
      @param entry[IN/OUT] contains a pointer to an entry that we can
                           use to search for another adjacent entry
@@ -121,9 +121,10 @@ public:
   bool next(HASH_ROW_POS_ENTRY** entry);
 
   /**
-     Deletes the entry pointed by entry. This is the only
-     safe way to free memory allocated for the structure
-     pointed to by entry.
+     Deletes the entry pointed by entry. It also frees memory used
+     holding entry contents. This is the way to release memeory 
+     used for entry, freeing it explicitly with my_free will cause
+     undefined behavior.
 
      @param entry  Pointer to the entry to be deleted.
      @returns true if something went wrong, false otherwise.

=== modified file 'sql/sys_vars.cc'
--- a/sql/sys_vars.cc	2010-11-23 00:51:55 +0000
+++ b/sql/sys_vars.cc	2011-01-26 16:00:19 +0000
@@ -1881,28 +1881,14 @@ static bool slave_rows_search_algorithms
 {
   String str, *res;
 
-  /**
-     'DEFAULT' values are not allowed.
-   */
   if(!var->value)
-    return true;
-
-  /**
-     NULL is not allowed.
-   */
-  if (check_not_null(self, thd, var))
-    return true;
+    return false;
 
   /** empty value ('') is not allowed */
   res= var->value->val_str(&str);
   if (res->is_empty())
     return true;
 
-  /** We don't allow only INDEX_SCAN to be set. */
-  if((res->length()==strlen("INDEX_SCAN")) && 
-     !strncasecmp(res->c_ptr_safe(), "index_scan", res->length()))
-    return true;
-
   return false;
 }
 


Attachment: [text/bzr-bundle] bzr/luis.soares@oracle.com-20110126160019-q2q37ns7r3py9fep.bundle
Thread
bzr commit into mysql-trunk-w5597 branch (luis.soares:3209) WL#5597Luis Soares26 Jan