2010-12-06 17:29, He Zhenxing wrote:
> Hi Daogang,
>
> Nice work, please look for some comments!
>
Thanks for your comments. See reply below.
> 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.
>
Yes. Updated.
>
>> +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;
>
Updated.
>
>> +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.
>
No. I don't think so. The sql_mode will affect its following transactions
and statements until it is updated. So I think we should save and restore
it so that it can keep consistency with master.
>
>> + 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.
>
If that, it will have different behavior if the sql_mode is added with
'MODE_STRICT_TRANS_TABLES' and 'MODE_STRICT_ALL_TABLES'.
Best Regards,
Daogang
>
>> // 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;);
>>
>>
>
>
>
>
