List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:August 23 2010 3:44am
Subject:Re: bzr commit into mysql-5.5-bugfixing branch (Dao-Gang.Qu:3192)
Bug#54579
View as plain text  
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.


Thread
bzr commit into mysql-5.5-bugfixing branch (Dao-Gang.Qu:3192) Bug#54579Dao-Gang.Qu22 Aug
  • Re: bzr commit into mysql-5.5-bugfixing branch (Dao-Gang.Qu:3192)Bug#54579He Zhenxing23 Aug
    • Re: bzr commit into mysql-5.5-bugfixing branch (Dao-Gang.Qu:3192)Bug#54579Daogang Qu23 Aug