bzr commit into mysql-pe branch (luis.soares:3829) Bug#49481 Bug#49482

[Luis Soares]

#At file:///home/lsoares/Workspace/bzr/work/bugfixing/49482/mysql-pe-push/ based on revid:joro@stripped

 3829 Luis Soares	2010-01-14 [merge]
      manual merge: Fix for BUG#49481 and BUG#49482 (5.1-bugteam --> mysql-pe).

    added:
      mysql-test/suite/rpl/r/rpl_myisam_null_values.result
      mysql-test/suite/rpl/t/rpl_myisam_null_values.test
    modified:
      sql/log_event.cc
      sql/log_event_old.cc
=== added file 'mysql-test/suite/rpl/r/rpl_myisam_null_values.result'
--- a/mysql-test/suite/rpl/r/rpl_myisam_null_values.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/rpl/r/rpl_myisam_null_values.result	2010-01-14 14:26:51 +0000
@@ -0,0 +1,24 @@
+stop slave;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+reset master;
+reset slave;
+drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
+start slave;
+CREATE TABLE t1 (c1 BIT, c2 INT);
+INSERT INTO `t1` VALUES ( 1, 1 );
+UPDATE t1 SET c1=NULL where c2=1;
+Comparing tables master:test.t1 and slave:test.t1
+DELETE FROM t1 WHERE c2=1 LIMIT 1;
+Comparing tables master:test.t1 and slave:test.t1
+DROP TABLE t1;
+CREATE TABLE t1 (c1 CHAR);
+INSERT INTO t1 ( c1 ) VALUES ( 'w' ) ;
+SELECT * FROM t1;
+c1
+w
+# should trigger switch to row due to LIMIT
+UPDATE t1 SET c1=NULL WHERE c1='w' LIMIT 2;
+Comparing tables master:test.t1 and slave:test.t1
+DELETE FROM t1 LIMIT 2;
+Comparing tables master:test.t1 and slave:test.t1
+DROP TABLE t1;

=== added file 'mysql-test/suite/rpl/t/rpl_myisam_null_values.test'
--- a/mysql-test/suite/rpl/t/rpl_myisam_null_values.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/rpl/t/rpl_myisam_null_values.test	2010-01-14 14:26:51 +0000
@@ -0,0 +1,53 @@
+# BUG#49481: RBR: MyISAM and bit fields may cause slave to stop on delete cant find record
+# BUG#49482: RBR: Replication may break on deletes when MyISAM tables + char field are used
+
+-- source include/master-slave.inc
+-- source include/have_binlog_format_mixed_or_row.inc
+
+-- connection master
+CREATE TABLE t1 (c1 BIT, c2 INT);
+INSERT INTO `t1` VALUES ( 1, 1 );
+UPDATE t1 SET c1=NULL where c2=1;
+-- sync_slave_with_master
+
+-- let $diff_table_1=master:test.t1
+-- let $diff_table_2=slave:test.t1
+-- source include/diff_tables.inc
+
+-- connection master
+DELETE FROM t1 WHERE c2=1 LIMIT 1;
+-- sync_slave_with_master
+
+-- let $diff_table_1=master:test.t1
+-- let $diff_table_2=slave:test.t1
+-- source include/diff_tables.inc
+
+-- connection master
+DROP TABLE t1;
+-- sync_slave_with_master
+
+-- connection master
+
+CREATE TABLE t1 (c1 CHAR);
+
+INSERT INTO t1 ( c1 ) VALUES ( 'w' ) ;
+SELECT * FROM t1;
+-- echo # should trigger switch to row due to LIMIT
+UPDATE t1 SET c1=NULL WHERE c1='w' LIMIT 2;
+-- sync_slave_with_master
+
+-- let $diff_table_1=master:test.t1
+-- let $diff_table_2=slave:test.t1
+-- source include/diff_tables.inc
+
+-- connection master
+DELETE FROM t1 LIMIT 2;
+-- sync_slave_with_master
+
+-- let $diff_table_1=master:test.t1
+-- let $diff_table_2=slave:test.t1
+-- source include/diff_tables.inc
+
+-- connection master
+DROP TABLE t1;
+-- sync_slave_with_master

=== modified file 'sql/log_event.cc'
--- a/sql/log_event.cc	2010-01-14 11:00:30 +0000
+++ b/sql/log_event.cc	2010-01-14 15:07:41 +0000
@@ -8792,6 +8792,24 @@ static bool record_compare(TABLE *table,
     }
   }
 
+  /**
+    Check if we are using MyISAM.
+
+    If this is a myisam table, then we cannot do a memcmp
+    right away because some NULL fields can still contain 
+    an old value in the row - they are not shown to the user
+    because the null bit is set, however, the contents are
+    not cleared. As such, plain memory comparison cannot be 
+    assured to work. See: BUG#49482 and BUG#49481.
+
+    On top of this, we do not store field contents for null 
+    fields in the binlog, so this is extra important when
+    comparing records fetched from binlog and from storage
+    engine.
+  */
+  if (table->file->ht->db_type == DB_TYPE_MYISAM)
+    goto record_compare_field_by_field;
+
   /* 
      We can compare the record straight away if:
       i) there are no blob and varchar fields
@@ -8816,17 +8834,36 @@ static bool record_compare(TABLE *table,
     goto record_compare_exit;
   }
 
+record_compare_field_by_field:
+
   /* Compare updated fields */
   for (Field **ptr=table->field ; *ptr ; ptr++)
   {
+    Field *f= *ptr;
     /* compare field if it is set */
-    if (bitmap_is_set(cols_in_readset, (*ptr)->field_index))
+    if (bitmap_is_set(cols_in_readset, f->field_index))
     { 
-      if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length))
+
+      /* if just one of the fields is null then there is no match */
+      if ((f->is_null_in_record(table->record[0])) ==
+           !(f->is_null_in_record(table->record[1])))
       {
         result= TRUE;
         goto record_compare_exit;
       }
+
+      /* if both fields are not null then we can compare */
+      if (!(f->is_null_in_record(table->record[0])) &&
+          !(f->is_null_in_record(table->record[1])))
+      {
+        if (f->cmp_binary_offset(table->s->rec_buff_length))
+        {
+          result= TRUE;
+          goto record_compare_exit;
+        }
+      }
+
+      /* if both fields are null then there is a match. compare next field */
     }
   }
 

=== modified file 'sql/log_event_old.cc'
--- a/sql/log_event_old.cc	2009-12-24 16:38:19 +0000
+++ b/sql/log_event_old.cc	2010-01-14 15:07:41 +0000
@@ -314,6 +314,24 @@ static bool record_compare(TABLE *table,
     }
   }
 
+  /**  
+    Check if we are using MyISAM.
+
+    If this is a myisam table, then we cannot do a memcmp
+    right away because some NULL fields can still contain 
+    an old value in the row - they are not shown to the user
+    because the null bit is set, however, the contents are
+    not cleared. As such, plain memory comparison cannot be 
+    assured to work. See: BUG#49482 and BUG#49481.
+
+    On top of this, we do not store field contents for null 
+    fields in the binlog, so this is extra important when
+    comparing records fetched from binlog and from storage
+    engine.
+  */
+  if (table->file->ht->db_type == DB_TYPE_MYISAM)
+    goto record_compare_field_by_field;
+
   /*   
      We can compare the record straight away if:
        i) there are no blob and varchar fields
@@ -338,17 +356,35 @@ static bool record_compare(TABLE *table,
     goto record_compare_exit;
   }
 
+record_compare_field_by_field:
+
   /* Compare updated fields */
   for (Field **ptr=table->field ; *ptr ; ptr++)
   {
+    Field *f= *ptr;
     /* compare field if it is set */
-    if (bitmap_is_set(cols_in_readset, (*ptr)->field_index))
+    if (bitmap_is_set(cols_in_readset, f->field_index))
     { 
-      if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length))
+      /* if just one of the fields is null then there is no match */
+      if ((f->is_null_in_record(table->record[0])) ==
+           !(f->is_null_in_record(table->record[1])))
       {
         result= TRUE;
         goto record_compare_exit;
       }
+
+      /* if both fields are not null then we can compare */
+      if (!(f->is_null_in_record(table->record[0])) &&
+          !(f->is_null_in_record(table->record[1])))
+      {
+        if (f->cmp_binary_offset(table->s->rec_buff_length))
+        {
+          result= TRUE;
+          goto record_compare_exit;
+        }
+      }
+
+      /* if both fields are null then there is a match. compare next field */
     }
   }
 


Attachment: [text/bzr-bundle] bzr/[e-mail]