List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:December 6 2010 9:29am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524)
Bug#56662
View as plain text  
Hi Daogang,

Nice work, please look for some comments!

Dao-Gang.Qu@stripped wrote:
> #At file:///home/daogang/bzrwork/bug56662/mysql-5.1-bugteam/ based on
> revid:bjorn.munch@stripped
> 
>  3524 Dao-Gang.Qu@stripped	2010-12-06
>       Bug #56662  	Assertion failed: next_insert_id == 0, file .\handler.cc
>       
>       In RBR, sometimes the new value of next_insert_id will be generated
>       for current row when updating the auto_increment field of a table on
>       slave side. Which causes the error. This is a incorrect behavior.
>       
>       After the patch, we never generate a new value for next_insert_id
>       for current row when updating the auto_increment field of a table
>       on slave side in RBR.
>      @ sql/log_event.cc
>         Added code to not allow generation of auto_increment value when
>         processing rows event by adding 'MODE_NO_AUTO_VALUE_ON_ZERO' to
>         sql_mode.
> 
>     modified:
>       mysql-test/extra/rpl_tests/rpl_auto_increment.test
>       mysql-test/suite/rpl/r/rpl_auto_increment.result
>       sql/log_event.cc
> === modified file 'mysql-test/extra/rpl_tests/rpl_auto_increment.test'
> --- a/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2009-09-10 10:05:53 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2010-12-06 05:37:52 +0000
> @@ -241,3 +241,29 @@ DROP TABLE t1;
>  DROP TABLE t2;
>  SET SQL_MODE='';
>  sync_slave_with_master;
> +
> +#
> +# BUG#56662
> +# The test verifies if the assertion of "next_insert_id == 0"
> +# will fail in ha_external_lock() function.
> +#
> +connection master;
> +CREATE TABLE t1 (id SMALLINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, data INT)
> ENGINE=innodb;
> +
> +BEGIN;
> +--echo # Set sql_mode without NO_AUTO_VALUE_ON_ZERO for 
> +--echo # allowing zero to fill the auto_increment field.
> +SET SQL_MODE=1400644414;

REQUEST: Use SET SQL_MODE='no_auto_value_on_zero';

This makes it clear that only this flag affects the test.

> +INSERT INTO t1(id,data) VALUES(0,2);
> +--echo # Resetting sql_mode with NO_AUTO_VALUE_ON_ZERO to
> +--echo # affect the execution of the transaction on slave.
> +SET SQL_MODE=1997596415;

REQUEST: SET SQL_MODE=0;

> +COMMIT;
> +SELECT * FROM t1;
> +sync_slave_with_master;
> +SELECT * FROM t1;
> +
> +connection master;
> +DROP TABLE t1;
> +sync_slave_with_master;
> +
> 
> === modified file 'mysql-test/suite/rpl/r/rpl_auto_increment.result'
> --- a/mysql-test/suite/rpl/r/rpl_auto_increment.result	2009-09-10 10:05:53 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_auto_increment.result	2010-12-06 05:37:52 +0000
> @@ -312,3 +312,20 @@ Comparing tables master:test.t2 and slav
>  DROP TABLE t1;
>  DROP TABLE t2;
>  SET SQL_MODE='';
> +CREATE TABLE t1 (id SMALLINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, data INT)
> ENGINE=innodb;
> +BEGIN;
> +# Set sql_mode without NO_AUTO_VALUE_ON_ZERO for 
> +# allowing zero to fill the auto_increment field.
> +SET SQL_MODE=1400644414;
> +INSERT INTO t1(id,data) VALUES(0,2);
> +# Resetting sql_mode with NO_AUTO_VALUE_ON_ZERO to
> +# affect the execution of the transaction on slave.
> +SET SQL_MODE=1997596415;
> +COMMIT;
> +SELECT * FROM t1;
> +id	data
> +0	2
> +SELECT * FROM t1;
> +id	data
> +0	2
> +DROP TABLE t1;
> 
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc	2010-10-23 12:55:44 +0000
> +++ b/sql/log_event.cc	2010-12-06 05:37:52 +0000
> @@ -7569,6 +7569,14 @@ int Rows_log_event::do_apply_event(Relay
>      // Do event specific preparations 
>      error= do_before_row_operations(rli);
>  
> +    /*
> +      Bug#56662 Assertion failed: next_insert_id == 0, file handler.cc
> +      Don't allow generation of auto_increment value when processing
> +      rows event by adding 'MODE_NO_AUTO_VALUE_ON_ZERO' to sql_mode.
> +    */
> +    ulong save_sql_mode= thd->variables.sql_mode;

SUGGESTION: I think it not necessary to save and restore the old value.

> +    thd->variables.sql_mode|= MODE_NO_AUTO_VALUE_ON_ZERO;
> +

SUGGESTION: I'd also suggest to use 
thd->variables.sql_mode= MODE_NO_AUTO_VALUE_ON_ZERO;

To make it clear that we only need this flag set when applying row
events.

>      // row processing loop
>  
>      while (error == 0 && m_curr_row < m_rows_end)
> @@ -7631,6 +7639,11 @@ int Rows_log_event::do_apply_event(Relay
>            thd->transaction.stmt.modified_non_trans_table= TRUE;
>      } // row processing loop
>  
> +    /*
> +      Restore the sql_mode after the rows event is processed.
> +    */
> +    thd->variables.sql_mode= save_sql_mode;
> +
>      DBUG_EXECUTE_IF("STOP_SLAVE_after_first_Rows_event",
>                      const_cast<Relay_log_info*>(rli)->abort_slave= 1;);
>  



Thread
bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524) Bug#56662Dao-Gang.Qu6 Dec
  • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524)Bug#56662He Zhenxing6 Dec
    • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524)Bug#56662Daogang Qu6 Dec