List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:December 29 2009 2:30am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3271)
Bug#49132
View as plain text  
Hi Daogang,

Great work.
However, the patch is not approved.

I have only one doubt about the patch before giving the green light.

There are several places in the code where the format is not set back.
For instance, "sql_acl.cc". Can you verify why we do not set the values back?

Please, fix the test case "binlog.binlog_row_mix_innodb_myisam" which
is failing.

Cheers.

Dao-Gang.Qu@stripped wrote:
> #At file:///home/daogangqu/mysql/bzrwork/bug49132/mysql-5.1-bugteam/ based on
> revid:alexey.kopytov@stripped
> 
>  3271 Dao-Gang.Qu@stripped	2009-12-22
>       Bug #49132  	Replication failure on temporary table + DDL
>       
>       In RBR, DDL statement will change binlog format to non row-based 
>       format, and then manipulating a temporary table can not reset binlog 
>       format to row-based format rightly. So that the manipulated statement  
>       is binlogged with statement-based format.
>       
>       To fix the problem, restore the state of binlog format after the DDL 
>       statement is binlogged.
>      @ mysql-test/suite/rpl/r/rpl_failure_on_tmp_table_and_DDL.result
>         Test result of the bug#49132.
>      @ mysql-test/suite/rpl/t/rpl_failure_on_tmp_table_and_DDL.test
>         Added the test file to verify if executing DDL statement 
>         before trying to manipulate a temporary table causes row-based 
>         replication to break with error 'table does not exist'.
> 
>     added:
>       mysql-test/suite/rpl/r/rpl_failure_on_tmp_table_and_DDL.result
>       mysql-test/suite/rpl/t/rpl_failure_on_tmp_table_and_DDL.test
>     modified:
>       sql/events.cc
>       sql/sp.cc
> === added file 'mysql-test/suite/rpl/r/rpl_failure_on_tmp_table_and_DDL.result'
> --- a/mysql-test/suite/rpl/r/rpl_failure_on_tmp_table_and_DDL.result	1970-01-01
> 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_failure_on_tmp_table_and_DDL.result	2009-12-22
> 10:18:53 +0000
> @@ -0,0 +1,42 @@
> +stop slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +reset master;
> +reset slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +start slave;
> +CREATE TEMPORARY TABLE t1 (a int);
> +CREATE EVENT e1 ON SCHEDULE EVERY 10 HOUR DO SELECT "nothing";
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +ALTER EVENT e1 ON SCHEDULE EVERY 20 HOUR DO SELECT "nothing";
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +DROP EVENT IF EXISTS e1;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +CREATE PROCEDURE p1() BEGIN SELECT 1 ;  END |
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +ALTER PROCEDURE p1 SQL SECURITY INVOKER;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +DROP PROCEDURE p1;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +CREATE FUNCTION f1() RETURNS INT RETURN 123;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +ALTER FUNCTION f1 SQL SECURITY INVOKER;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +CREATE TEMPORARY TABLE t1 (a int);
> +DROP FUNCTION f1;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> 
> === added file 'mysql-test/suite/rpl/t/rpl_failure_on_tmp_table_and_DDL.test'
> --- a/mysql-test/suite/rpl/t/rpl_failure_on_tmp_table_and_DDL.test	1970-01-01
> 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_failure_on_tmp_table_and_DDL.test	2009-12-22
> 10:18:53 +0000
> @@ -0,0 +1,68 @@
> +#
> +# Bug#49132
> +# This test verify if executing DDL statement before trying to 
> +# manipulate a temporary table causes row-based replication to 
> +# break with error 'table does not exist'.
> +#
> +--source include/master-slave.inc
> +--source include/have_binlog_format_row.inc
> +
> +# CREATE EVENT when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +CREATE EVENT e1 ON SCHEDULE EVERY 10 HOUR DO SELECT "nothing";
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +# ALTER EVENT when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +ALTER EVENT e1 ON SCHEDULE EVERY 20 HOUR DO SELECT "nothing";
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +# DROP EVENT when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +DROP EVENT IF EXISTS e1;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +# CREATE PROCEDURE when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +DELIMITER |;
> +CREATE PROCEDURE p1() BEGIN SELECT 1 ;  END |
> +DELIMITER ;|
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +# Alter PROCEDURE when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +ALTER PROCEDURE p1 SQL SECURITY INVOKER;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +
> +# DROP PROCEDURE when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +DROP PROCEDURE p1;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +# CREATE FUNCTION when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +CREATE FUNCTION f1() RETURNS INT RETURN 123;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +# ALTER FUNCTION when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +ALTER FUNCTION f1 SQL SECURITY INVOKER;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +# DROP FUNCTION when a temporary table is open.
> +CREATE TEMPORARY TABLE t1 (a int);
> +DROP FUNCTION f1;
> +insert into t1 values (1);
> +DROP TEMPORARY TABLE t1;
> +
> +--sync_slave_with_master
> +
> 
> === modified file 'sql/events.cc'
> --- a/sql/events.cc	2009-10-16 10:29:42 +0000
> +++ b/sql/events.cc	2009-12-22 10:18:53 +0000
> @@ -479,6 +479,9 @@ Events::create_event(THD *thd, Event_par
>      }
>    }
>    pthread_mutex_unlock(&LOCK_event_metadata);
> +  /* Restore the state of binlog format */
> +  if (thd->variables.binlog_format == BINLOG_FORMAT_ROW)
> +    thd->current_stmt_binlog_row_based= TRUE;
>  
>    DBUG_RETURN(ret);
>  }
> @@ -600,6 +603,9 @@ Events::update_event(THD *thd, Event_par
>      }
>    }
>    pthread_mutex_unlock(&LOCK_event_metadata);
> +  /* Restore the state of binlog format */
> +  if (thd->variables.binlog_format == BINLOG_FORMAT_ROW)
> +    thd->current_stmt_binlog_row_based= TRUE;
>  
>    DBUG_RETURN(ret);
>  }
> @@ -674,6 +680,9 @@ Events::drop_event(THD *thd, LEX_STRING 
>      write_bin_log(thd, TRUE, thd->query(), thd->query_length());
>    }
>    pthread_mutex_unlock(&LOCK_event_metadata);
> +  /* Restore the state of binlog format */
> +  if (thd->variables.binlog_format == BINLOG_FORMAT_ROW)
> +    thd->current_stmt_binlog_row_based= TRUE; 
>    DBUG_RETURN(ret);
>  }
>  
> 
> === modified file 'sql/sp.cc'
> --- a/sql/sp.cc	2009-12-02 11:17:08 +0000
> +++ b/sql/sp.cc	2009-12-22 10:18:53 +0000
> @@ -1118,6 +1118,9 @@ done:
>    thd->variables.sql_mode= saved_mode;
>  
>    close_thread_tables(thd);
> +  /* Restore the state of binlog format */
> +  if (thd->variables.binlog_format == BINLOG_FORMAT_ROW)
> +    thd->current_stmt_binlog_row_based= TRUE;
>    DBUG_RETURN(ret);
>  }
>  
> @@ -1171,6 +1174,9 @@ sp_drop_routine(THD *thd, int type, sp_n
>    }
>  
>    close_thread_tables(thd);
> +  /* Restore the state of binlog format */
> +  if (thd->variables.binlog_format == BINLOG_FORMAT_ROW)
> +    thd->current_stmt_binlog_row_based= TRUE;
>    DBUG_RETURN(ret);
>  }
>  
> @@ -1241,6 +1247,9 @@ sp_update_routine(THD *thd, int type, sp
>    }
>  
>    close_thread_tables(thd);
> +  /* Restore the state of binlog format */
> +  if (thd->variables.binlog_format == BINLOG_FORMAT_ROW)
> +    thd->current_stmt_binlog_row_based= TRUE;
>    DBUG_RETURN(ret);
>  }
>  
> 
> 
> 
> ------------------------------------------------------------------------
> 
> 
Thread
bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3271) Bug#49132Dao-Gang.Qu22 Dec
  • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3271)Bug#49132Alfranio Correia29 Dec
    • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3271)Bug#49132Daogang Qu29 Dec