From: Alfranio Correia
Date: March 25 2011 11:36am
Subject: Re: bzr commit into mysql-trunk branch (luis.soares:3323) Bug#11766865
List-Archive: http://lists.mysql.com/commits/133866
Message-Id: <4D8C7E52.9000404@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Hi Luis,

Great work.

Patch approved.


On 03/24/2011 02:15 PM, Luis Soares wrote:
> #At file:///home/lsoares/Workspace/bzr/work/bugfixing/11766865/push/mysql-trunk/ based on revid:olav.sandstaa@stripped
> 
>  3323 Luis Soares	2011-03-24 [merge]
>       BUG#11766865: Manual merge from mysql-5.5 into latest mysql-trunk.
>       
>       Conflicts
>       =========
>         sql/log_event.cc
> 
>     modified:
>       mysql-test/extra/rpl_tests/rpl_record_compare.test
>       mysql-test/suite/rpl/r/rpl_row_rec_comp_innodb.result
>       mysql-test/suite/rpl/r/rpl_row_rec_comp_myisam.result
>       mysql-test/suite/rpl/t/rpl_row_rec_comp_myisam.test
>       sql/log_event.cc
> === modified file 'mysql-test/extra/rpl_tests/rpl_record_compare.test'
> --- a/mysql-test/extra/rpl_tests/rpl_record_compare.test	2010-12-19 17:07:28 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_record_compare.test	2011-03-24 10:52:40 +0000
> @@ -62,4 +62,24 @@ UPDATE t1 SET c1= 0;
>  DROP TABLE t1; 
>  -- sync_slave_with_master
>  
> +#
> +# BUG#11766865: 60091: RBR + NO PK + UPDATE NULL VALUE --> SLAVE BREAK WITH ERROR HA_ERR_END_OF_ 
> +#
>  
> +--connection master
> +--source include/rpl_reset.inc
> +--connection master
> +
> +--eval CREATE TABLE t1 (c1 int(11) NOT NULL, c2 int(11) NOT NULL, c3 int(11) DEFAULT '-1') ENGINE=$engine DEFAULT CHARSET=latin1
> +
> +INSERT INTO t1 VALUES (1,2,NULL);
> +UPDATE t1 SET c1=1, c2=2, c3=-1 WHERE c1=1 AND c2=2 AND ISNULL(c3);
> +
> +--sync_slave_with_master
> +
> +--let $diff_tables=master:test.t1, slave:test.t1
> +--source include/diff_tables.inc
> +
> +--connection master
> +DROP TABLE t1;
> +--sync_slave_with_master
> 
> === modified file 'mysql-test/suite/rpl/r/rpl_row_rec_comp_innodb.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_rec_comp_innodb.result	2010-12-19 17:07:28 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_rec_comp_innodb.result	2011-03-24 10:52:40 +0000
> @@ -25,4 +25,10 @@ INSERT INTO t1(c1) VALUES (NULL);
>  UPDATE t1 SET c1= 0;
>  include/diff_tables.inc [master:t1, slave:t1]
>  DROP TABLE t1;
> +include/rpl_reset.inc
> +CREATE TABLE t1 (c1 int(11) NOT NULL, c2 int(11) NOT NULL, c3 int(11) DEFAULT '-1') ENGINE=InnoDB DEFAULT CHARSET=latin1;
> +INSERT INTO t1 VALUES (1,2,NULL);
> +UPDATE t1 SET c1=1, c2=2, c3=-1 WHERE c1=1 AND c2=2 AND ISNULL(c3);
> +include/diff_tables.inc [master:test.t1, slave:test.t1]
> +DROP TABLE t1;
>  include/rpl_end.inc
> 
> === modified file 'mysql-test/suite/rpl/r/rpl_row_rec_comp_myisam.result'
> --- a/mysql-test/suite/rpl/r/rpl_row_rec_comp_myisam.result	2010-12-19 17:07:28 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_row_rec_comp_myisam.result	2011-03-24 10:52:40 +0000
> @@ -1,5 +1,14 @@
>  include/master-slave.inc
>  [connection master]
> +## coverage purposes - Field_bits 
> +##                     1 X bit + 2 Null bits + 5 bits => last_null_bit_pos==0
> +include/rpl_reset.inc
> +CREATE TABLE t1 (c1 bigint(20) DEFAULT 0, c2 bit(5)) ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t1(c1,c2) VALUES (10, b'1');
> +INSERT INTO t1(c1,c2) VALUES (NULL, b'1');
> +UPDATE t1 SET c1= 0;
> +include/diff_tables.inc [master:t1, slave:t1]
> +DROP TABLE t1;
>  ## case #1 - last_null_bit_pos==0 in record_compare without X bit
>  include/rpl_reset.inc
>  CREATE TABLE t1 (c1 bigint(20) DEFAULT 0, c2 bigint(20) DEFAULT 0, c3 bigint(20) DEFAULT 0, c4 varchar(1) DEFAULT '', c5 bigint(20) DEFAULT 0, c6 bigint(20) DEFAULT 0, c7 bigint(20) DEFAULT 0, c8 bigint(20) DEFAULT 0) ENGINE=MyISAM DEFAULT CHARSET=latin1;
> @@ -25,13 +34,10 @@ INSERT INTO t1(c1) VALUES (NULL);
>  UPDATE t1 SET c1= 0;
>  include/diff_tables.inc [master:t1, slave:t1]
>  DROP TABLE t1;
> -## coverage purposes - Field_bits 
> -##                     1 X bit + 2 Null bits + 5 bits => last_null_bit_pos==0
>  include/rpl_reset.inc
> -CREATE TABLE t1 (c1 bigint(20) DEFAULT 0, c2 bit(5)) ENGINE=MyISAM DEFAULT CHARSET=latin1;
> -INSERT INTO t1(c1,c2) VALUES (10, b'1');
> -INSERT INTO t1(c1,c2) VALUES (NULL, b'1');
> -UPDATE t1 SET c1= 0;
> -include/diff_tables.inc [master:t1, slave:t1]
> +CREATE TABLE t1 (c1 int(11) NOT NULL, c2 int(11) NOT NULL, c3 int(11) DEFAULT '-1') ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +INSERT INTO t1 VALUES (1,2,NULL);
> +UPDATE t1 SET c1=1, c2=2, c3=-1 WHERE c1=1 AND c2=2 AND ISNULL(c3);
> +include/diff_tables.inc [master:test.t1, slave:test.t1]
>  DROP TABLE t1;
>  include/rpl_end.inc
> 
> === modified file 'mysql-test/suite/rpl/t/rpl_row_rec_comp_myisam.test'
> --- a/mysql-test/suite/rpl/t/rpl_row_rec_comp_myisam.test	2010-12-19 17:07:28 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_row_rec_comp_myisam.test	2011-03-24 10:52:40 +0000
> @@ -1,12 +1,11 @@
>  -- source include/have_binlog_format_row.inc
>  -- source include/master-slave.inc
> +-- let $engine= MyISAM
>  
>  #
>  # BUG#52868 Wrong handling of NULL value during update, replication out of sync
>  #
>  
> --- let $engine= MyISAM
> --- source extra/rpl_tests/rpl_record_compare.test
>  
>  -- echo ## coverage purposes - Field_bits 
>  -- echo ##                     1 X bit + 2 Null bits + 5 bits => last_null_bit_pos==0
> @@ -28,4 +27,7 @@ UPDATE t1 SET c1= 0;
>  -- connection master
>  DROP TABLE t1; 
>  -- sync_slave_with_master
> +
> +-- source extra/rpl_tests/rpl_record_compare.test
> +
>  --source include/rpl_end.inc
> 
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc	2011-03-24 08:00:03 +0000
> +++ b/sql/log_event.cc	2011-03-24 14:15:41 +0000
> @@ -9425,39 +9425,52 @@ static bool record_compare(TABLE *table,
>      }
>    }
>  
> -  if (table->s->blob_fields + table->s->varchar_fields == 0 &&
> +  /**
> +    Compare full record only if:
> +    - there are no blob fields (otherwise we would also need 
> +      to compare blobs contents as well);
> +    - there are no varchar fields (otherwise we would also need
> +      to compare varchar contents as well);
> +    - there are no null fields, otherwise NULLed fields 
> +      contents (i.e., the don't care bytes) may show arbitrary 
> +      values, depending on how each engine handles internally.
> +    - if all the bitmap is set (both are full rows)
> +    */
> +  if ((table->s->blob_fields + 
> +       table->s->varchar_fields +
> +       table->s->null_fields) == 0 &&
>        bitmap_is_set_all(cols))
>    {
>      result= cmp_record(table,record[1]);
> -    goto record_compare_exit;
>    }
> -
> -  /* Compare null bits */
> -  if (bitmap_is_set_all(cols) &&
> -      memcmp(table->null_flags,
> -	     table->null_flags+table->s->rec_buff_length,
> -	     table->s->null_bytes))
> -  {
> -    result= TRUE;				// Diff in NULL value
> -    goto record_compare_exit;
> -  }
> -
> -  /* Compare updated fields */
> -  for (Field **ptr=table->field ; 
> -       *ptr && ((*ptr)->field_index < cols->n_bits);
> -       ptr++)
> +  else
>    {
> -    if (bitmap_is_set(cols, (*ptr)->field_index))
> +    /* 
> +      Fallback to field-by-field comparison:
> +      1. start by checking if the field is signaled:
> +      2. if it is, first compare the null bit if the field is nullable
> +      3. then compare the contents of the field, if it is not 
> +         set to null
> +     */
> +    for (Field **ptr=table->field ; 
> +         *ptr && ((*ptr)->field_index < cols->n_bits) && !result;
> +         ptr++)
>      {
> -      if ((*ptr)->cmp_binary_offset(table->s->rec_buff_length))
> +      Field *field= *ptr;
> +
> +      if (bitmap_is_set(cols, field->field_index))
>        {
> -        result= TRUE;
> -        goto record_compare_exit;
> +        /* compare null bit */
> +        if (field->is_null() != field->is_null_in_record(table->record[1]))
> +          result= TRUE;
> +
> +        /* compare content, only if fields are not set to NULL */
> +        else if (!field->is_null())
> +          result= field->cmp_binary_offset(table->s->rec_buff_length);
>        }
>      }
>    }
>  
> -record_compare_exit:
>    /*
>      Restore the saved bytes.
>  
> 
> No bundle (reason: revision is a merge (you can force generation of a bundle with env var BZR_FORCE_BUNDLE=1)).
>