From a0a5152fb316f752212c6559b72464dde1f3fbd6 Mon Sep 17 00:00:00 2001 From: Luis Soares Date: Thu, 14 Jan 2010 14:26:51 +0000 Subject: [PATCH] Fix for BUG#49481 and BUG#49482. 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 When using MyISAM tables, despite the fact that the null bit is set for some fields, their old value is still in the row. This can cause the comparison of records to fail when the slave is doing an index or range scan. We fix this by avoiding memcmp for MyISAM tables when comparing records. Additionally, when comparing field by field, we first check if both fields are not null and if so, then we compare them. If just one field is null we return failure immediately. If both fields are null, we move on to the next field. --- .../suite/rpl/r/rpl_myisam_null_values.result | 24 +++++++++ .../suite/rpl/t/rpl_myisam_null_values.test | 53 +++++++++++++++++++ sql/log_event.cc | 39 +++++++++++++- sql/log_event_old.cc | 39 +++++++++++++- 4 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_myisam_null_values.result create mode 100644 mysql-test/suite/rpl/t/rpl_myisam_null_values.test diff --git a/mysql-test/suite/rpl/r/rpl_myisam_null_values.result b/mysql-test/suite/rpl/r/rpl_myisam_null_values.result new file mode 100644 index 00000000000..574528a8d79 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_myisam_null_values.result @@ -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; diff --git a/mysql-test/suite/rpl/t/rpl_myisam_null_values.test b/mysql-test/suite/rpl/t/rpl_myisam_null_values.test new file mode 100644 index 00000000000..d9ec95fc510 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_myisam_null_values.test @@ -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 diff --git a/sql/log_event.cc b/sql/log_event.cc index 6b21087b6aa..61f5e93e29e 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -8724,6 +8724,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; + if (table->s->blob_fields + table->s->varchar_fields == 0) { result= cmp_record(table,record[1]); @@ -8739,14 +8757,33 @@ 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++) { - if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length)) + Field *f= *ptr; + + /* 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 */ } record_compare_exit: diff --git a/sql/log_event_old.cc b/sql/log_event_old.cc index 357bc78b1cd..72a6e159535 100644 --- a/sql/log_event_old.cc +++ b/sql/log_event_old.cc @@ -351,6 +351,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; + if (table->s->blob_fields + table->s->varchar_fields == 0) { result= cmp_record(table,record[1]); @@ -366,14 +384,33 @@ 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++) { - if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length)) + Field *f= *ptr; + + /* 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 */ } record_compare_exit: