Hi Libing,
Thanks for the good comments, Please review the newest patch. Thanks!
Best Regards,
Daogang
Libing Song wrote:
> Hi Daogang,
> Nice work. There are only some minor comments, please see below!
>
> Dao-Gang.Qu@stripped wrote:
>> #At file:///home/daogangq/mysql/bzrwork/bug45827/mysql-pe/ based on
>> revid:luis.soares@stripped
>>
>> 3605 Dao-Gang.Qu@stripped 2009-09-16
>> Bug #45827 Stmt using two autoinc values does not produce
>> unsafe warning
>> One statement that have more than one different tables to
>> update with autoinc columns just was marked as unsafe in mixed
>> mode, so the unsafe warning can't be produced in statement mode.
>> To fix the problem, mark the statement as unsafe in
>> statement and row mode too.
>>
> why is row mode same as statement.
> Maybe you mean:
> " mark the statement as unsafe. Unsafe warning will be reported if it
> is in SBR,
> else both in MBR and RBR, the statement will be binlogged in row
> mode. so, it is safe. "
>
>> @ mysql-test/suite/binlog/r/binlog_unsafe.result
>> Test result for bug#45827
>> @ mysql-test/suite/binlog/t/binlog_unsafe.test
>> Added test to verify if stmt that have more than one
>> different tables to update with autoinc columns will
>> produce unsafe warning
>> @ sql/sql_base.cc
>> Get rid of the check of BINLOG_FORMAT_MIXED
>>
>> modified:
>> mysql-test/suite/binlog/r/binlog_unsafe.result
>> mysql-test/suite/binlog/t/binlog_unsafe.test
>> sql/sql_base.cc
>> === modified file 'mysql-test/suite/binlog/r/binlog_unsafe.result'
>> --- a/mysql-test/suite/binlog/r/binlog_unsafe.result 2009-07-28
>> 14:16:37 +0000
>> +++ b/mysql-test/suite/binlog/r/binlog_unsafe.result 2009-09-16
>> 15:35:16 +0000
>> @@ -300,4 +300,59 @@ Warnings:
>> Note 1592 Statement may not be safe to log in statement format.
>> DROP TABLE t1, t2;
>> SET @@SESSION.SQL_MODE = @save_sql_mode;
>> +CREATE TABLE t1 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +CREATE TABLE t2 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +CREATE FUNCTION func_modify_t1 ()
>> +RETURNS INT
>> +BEGIN
>> +INSERT INTO t1 SET a = 1;
>> +RETURN 0;
>> +END|
>> +# The following statement causes auto-incrementation +# of both t1
>> and t2. It is logged in statement format, +# so it should produce
>> unsafe warning.
>> +INSERT INTO t2 SET a = func_modify_t1();
>> +Warnings:
>> +Note 1592 Statement may not be safe to log in statement format.
>> +SET SESSION binlog_format = MIXED;
>> +# Check if the statement is logged in row format.
>> +INSERT INTO t2 SET a = func_modify_t1();
>> +SHOW BINLOG EVENTS FROM 12283;
>> +Log_name Pos Event_type Server_id End_log_pos Info
>> +master-bin.000001 12283 Query 1 12351 BEGIN
>> +master-bin.000001 12351 Table_map 1 12393 table_id:
>> 43 (test.t2)
>> +master-bin.000001 12393 Table_map 1 12435 table_id:
>> 44 (test.t1)
>> +master-bin.000001 12435 Write_rows 1 12473 table_id: 44
>> +master-bin.000001 12473 Write_rows 1 12511 table_id:
>> 43 flags: STMT_END_F
>> +master-bin.000001 12511 Query 1 12580 COMMIT
>> +DROP TABLE t1,t2;
>> +DROP FUNCTION func_modify_t1;
>> +SET SESSION binlog_format = STATEMENT;
>> +CREATE TABLE t1 (a INT);
>> +CREATE TABLE t2 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +CREATE TABLE t3 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +create trigger tri_modify_two_tables before insert on t1 for each
>> row begin
>> +insert into t2(a) values(new.a);
>> +insert into t3(a) values(new.a);
>> +end |
>> +# The following statement causes auto-incrementation +# of both t2
>> and t3. It is logged in statement format, +# so it should produce
>> unsafe warning
>> +INSERT INTO t1 SET a = 1;
>> +Warnings:
>> +Note 1592 Statement may not be safe to log in statement format.
>> +SET SESSION binlog_format = MIXED;
>> +# Check if the statement is logged in row format.
>> +INSERT INTO t1 SET a = 2;
>> +SHOW BINLOG EVENTS FROM 13426;
>> +Log_name Pos Event_type Server_id End_log_pos Info
>> +master-bin.000001 13426 Query 1 13494 BEGIN
>> +master-bin.000001 13494 Table_map 1 13535 table_id:
>> 46 (test.t1)
>> +master-bin.000001 13535 Table_map 1 13577 table_id:
>> 47 (test.t3)
>> +master-bin.000001 13577 Table_map 1 13619 table_id:
>> 48 (test.t2)
>> +master-bin.000001 13619 Write_rows 1 13657 table_id: 48
>> +master-bin.000001 13657 Write_rows 1 13695 table_id: 47
>> +master-bin.000001 13695 Write_rows 1 13729 table_id:
>> 46 flags: STMT_END_F
>> +master-bin.000001 13729 Query 1 13798 COMMIT
>> +DROP TABLE t1,t2,t3;
>> "End of tests"
>>
>> === modified file 'mysql-test/suite/binlog/t/binlog_unsafe.test'
>> --- a/mysql-test/suite/binlog/t/binlog_unsafe.test 2009-07-28
>> 14:16:37 +0000
>> +++ b/mysql-test/suite/binlog/t/binlog_unsafe.test 2009-09-16
>> 15:35:16 +0000
>> @@ -47,6 +47,7 @@
>> # BUG#34768: nondeterministic INSERT using LIMIT logged in stmt mode
>> if binlog_format=mixed
>> # BUG#41980, SBL, INSERT .. SELECT .. LIMIT = ERROR, even when
>> @@SQL_LOG_BIN is 0
>> # BUG#42640: mysqld crashes when unsafe statements are executed
>> (STRICT_TRANS_TABLES mode)
>> +# Bug#45827: Stmt using two autoinc values does not produce unsafe
>> warning
>> #
>> # ==== Related test cases ====
>> #
>> @@ -57,6 +58,8 @@
>> # rpl.rpl_variables_stm tests the small subset of variables that
>> # actually can be replicated safely in statement mode.
>> #
>> +# Tests if stmt that have more than one different tables to update
>> +# with autoinc columns will produce unsafe warning.
>> #
>> # ==== Todo ====
>> #
>> @@ -388,4 +391,73 @@ DELETE FROM t1 LIMIT 1;
>>
>> DROP TABLE t1, t2;
>> SET @@SESSION.SQL_MODE = @save_sql_mode;
>> +
>> +#
>> +# BUG#45827
>> +# The test verifies if stmt that have more than one +# different
>> tables to update with autoinc columns +# will produce unsafe warning
>> +#
>> +
>> +# Test case1: stmt that have more than one different tables
>> +# to update with autoinc columns should produce
>> +# unsafe warning when calling a function
>> +CREATE TABLE t1 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +CREATE TABLE t2 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +
>> +# The purpose of this function is to insert into t1 so that the second
>> +# column is auto_increment'ed.
>> +DELIMITER |;
>> +CREATE FUNCTION func_modify_t1 ()
>> +RETURNS INT
>> +BEGIN
>> + INSERT INTO t1 SET a = 1;
>> + RETURN 0;
>> +END|
>> +DELIMITER ;|
>> +--echo # The following statement causes auto-incrementation +--echo
>> # of both t1 and t2. It is logged in statement format, +--echo # so
>> it should produce unsafe warning.
>> +INSERT INTO t2 SET a = func_modify_t1();
>> +
>> +SET SESSION binlog_format = MIXED;
>> +--echo # Check if the statement is logged in row format.
>> +let $pos0_master= query_get_value(SHOW MASTER STATUS, Position, 1);
>> +INSERT INTO t2 SET a = func_modify_t1();
>> +eval SHOW BINLOG EVENTS FROM $pos0_master;
>> +
>> +# clean up
>> +DROP TABLE t1,t2;
>> +DROP FUNCTION func_modify_t1;
>> +
>> +# Test case2: stmt that have more than one different tables
>> +# to update with autoinc columns should produce
>> +# unsafe warning when invoking a trigger +SET SESSION
>> binlog_format = STATEMENT;
>> +CREATE TABLE t1 (a INT);
>> +CREATE TABLE t2 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +CREATE TABLE t3 (a INT, b INT PRIMARY KEY AUTO_INCREMENT);
>> +
>> +# The purpose of this function is to insert into t1 so that the second
>> +# column is auto_increment'ed.
>> +delimiter |;
>> +create trigger tri_modify_two_tables before insert on t1 for each
>> row begin
>> + insert into t2(a) values(new.a);
>> + insert into t3(a) values(new.a);
>> +end |
>> +delimiter ;|
>> +--echo # The following statement causes auto-incrementation +--echo
>> # of both t2 and t3. It is logged in statement format, +--echo # so
>> it should produce unsafe warning
>> +INSERT INTO t1 SET a = 1;
>> +
>> +SET SESSION binlog_format = MIXED;
>> +--echo # Check if the statement is logged in row format.
>> +let $pos1_master= query_get_value(SHOW MASTER STATUS, Position, 1);
>> +INSERT INTO t1 SET a = 2;
>> +eval SHOW BINLOG EVENTS FROM $pos1_master;
>> +
>> +# clean up
>> +DROP TABLE t1,t2,t3;
>> +
>> --echo "End of tests"
>>
>> === modified file 'sql/sql_base.cc'
>> --- a/sql/sql_base.cc 2009-09-11 10:45:04 +0000
>> +++ b/sql/sql_base.cc 2009-09-16 15:35:16 +0000
>> @@ -4992,11 +4992,9 @@ bool lock_tables(THD *thd, TABLE_LIST *t
>> statement-based binlogging won't work. We can solve this
>> problem in
>> mixed mode by switching to row-based binlogging:
>> */
>>
> The comment above is out of date, It should be changed.
Good.
>> - if (thd->variables.binlog_format == BINLOG_FORMAT_MIXED &&
>> - has_two_write_locked_tables_with_auto_increment(tables))
>> + if (has_two_write_locked_tables_with_auto_increment(tables))
>> {
>> thd->lex->set_stmt_unsafe();
>> - thd->set_current_stmt_binlog_row_based_if_mixed();
>> }
>> }
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>
