Hi Daogang,
Thank you for addressing the comments, but there is still a problem in
the patch, please look below for the detail.
Dao-Gang.Qu@stripped wrote:
> #At file:///home/daogangqu/mysql/bzrwork1/bug54579/mysql-5.5-bugfixing/ based on
> revid:alfranio.correia@stripped
>
> 3192 Dao-Gang.Qu@stripped 2010-08-22
> Bug #54579 Wrong unsafe warning for INSERT DELAYED in SBR
>
> The lock_type is upgrade to TL_WRITE from TL_WRITE_DELAYED for
> INSERT DELAYED when inserting multi values in one statement.
> It's safe. But it causes an unsafe warning in SBR.
>
> Make INSERT DELAYED safe by logging it as INSERT without DELAYED.
> @ mysql-test/extra/binlog_tests/binlog_insert_delayed.test
> Updated the test file to test multi INSERT DELAYED statement
> is no longer causes an unsafe warning and binlogged as INSERT
> without DELAYED.
> @ mysql-test/extra/rpl_tests/create_recursive_construct.inc
> Updated for the patch of bug#54579.
> @ mysql-test/suite/binlog/r/binlog_row_binlog.result
> Updated for the patch of bug#54579.
> @ mysql-test/suite/binlog/r/binlog_statement_insert_delayed.result
> Test result for BUG#54579.
> @ mysql-test/suite/binlog/r/binlog_stm_binlog.result
> Updated for the patch of bug#54579.
> @ mysql-test/suite/binlog/r/binlog_unsafe.result
> Updated for the patch of bug#54579.
> @ mysql-test/suite/binlog/t/binlog_unsafe.test
> Updated for the patch of bug#54579.
> @ sql/sql_insert.cc
> Added code to genetate a new query string for removing
> DELAYED keyword for multi INSERT DEALAYED statement.
> @ sql/sql_yacc.yy
> Added code to record the DELAYED keyword position and remove the setting
> of unsafe statement for INSERT DELAYED statement
>
> modified:
> mysql-test/extra/binlog_tests/binlog_insert_delayed.test
> mysql-test/extra/rpl_tests/create_recursive_construct.inc
> mysql-test/suite/binlog/r/binlog_row_binlog.result
> mysql-test/suite/binlog/r/binlog_statement_insert_delayed.result
> mysql-test/suite/binlog/r/binlog_stm_binlog.result
> mysql-test/suite/binlog/r/binlog_unsafe.result
> mysql-test/suite/binlog/t/binlog_unsafe.test
> sql/sql_insert.cc
> sql/sql_lex.h
> sql/sql_yacc.yy
> === modified file 'mysql-test/extra/binlog_tests/binlog_insert_delayed.test'
> --- a/mysql-test/extra/binlog_tests/binlog_insert_delayed.test 2008-07-23 16:56:39
> +0000
> +++ b/mysql-test/extra/binlog_tests/binlog_insert_delayed.test 2010-08-22 07:42:32
> +0000
> @@ -46,19 +46,19 @@ insert delayed into t1 values (300);
> inc $count;
> --source include/wait_until_rows_count.inc
>
> -# It is not enough to wait until all rows have been inserted into the
> -# table. FLUSH TABLES ensures that they are in the binlog too. See
> -# comment above.
> -FLUSH TABLES;
> -source include/show_binlog_events.inc;
> -
> -insert delayed into t1 values (null),(null),(null),(null);
> +insert /* before delayed */ delayed /* after delayed */ into t1 values
> (null),(null),(null),(null);
> inc $count; inc $count; inc $count; inc $count;
> --source include/wait_until_rows_count.inc
>
> -insert delayed into t1 values (null),(null),(400),(null);
> +insert /* before delayed */ delayed /* after delayed */ into t1 values
> (null),(null),(400),(null);
> inc $count; inc $count; inc $count; inc $count;
> --source include/wait_until_rows_count.inc
>
> +# It is not enough to wait until all rows have been inserted into the
> +# table. FLUSH TABLES ensures that they are in the binlog too. See
> +# comment above.
> +FLUSH TABLES;
> +source include/show_binlog_events.inc;
> +
> select * from t1;
> drop table t1;
>
> === modified file 'mysql-test/extra/rpl_tests/create_recursive_construct.inc'
> --- a/mysql-test/extra/rpl_tests/create_recursive_construct.inc 2010-08-10 11:32:54
> +0000
> +++ b/mysql-test/extra/rpl_tests/create_recursive_construct.inc 2010-08-22 07:42:32
> +0000
> @@ -341,7 +341,7 @@ if (`SELECT '$CRC_RET_stmt_sidef' != ''`
> # The first event is format_description, the second is
> # Query_event('BEGIN'), and the third should be our Table_map.
> --let $event_type= query_get_value(SHOW BINLOG EVENTS, Event_type, 3)
> - if (`SELECT '$event_type' != 'Table_map'`) {
> + if (`SELECT $unsafe_type != 3 AND '$event_type' != 'Table_map'`) {
The original purpose of this check is to make sure the statement is
binlogged in ROW format, the modification above will disable the check,
which is not correct.
I think you should either remove the INSERT DELAYED statement from the
test case. or add code to check that for INSERT DELAYED, the third event
is a Query, and for others, they are Table_map.
