From: Dmitry Lenev
Date: March 5 2011 12:07pm
Subject: Re: bzr commit into mysql-5.5 branch (jon.hauglid:3371)
 Bug#11765416
List-Archive: http://lists.mysql.com/commits/132518
Message-Id: <20110305120759.GB28737@bandersnatch>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Hi, Jon Olav!

Here are my comments about your patch:

* Jon Olav Hauglid <jon.hauglid@stripped> [11/03/04 18:32]:
> #At file:///export/home/x/mysql-5.5-bug11765416/ based on revid:build@stripped
> 
>  3371 Jon Olav Hauglid	2011-03-04
>       Bug #11765416 (former 58381)
>       FAILED DROP DATABASE CAN BREAK STATEMENT BASED REPLICATION
>       
>       The first phase of DROP DATABASE is to delete the tables in the database.
>       If deletion of one or more of the tables fail (e.g. due to a FOREIGN KEY
>       constraint), DROP DATABASE will be aborted. However, some tables could
>       still have been deleted. The problem was that nothing would be written
>       to the binary log in this case, so any slaves would not delete these tables.
>       Therefore the master and the slaves would get out of sync.
>       
>       This patch fixes the problem by making sure that DROP TABLE is written
>       to the binary log for the tables that were in fact deleted by the failed
>       DROP DATABASE statement.
>       
>       Test case added to mysqlbinlog.test.

...

> === modified file 'mysql-test/t/mysqlbinlog.test'
> --- a/mysql-test/t/mysqlbinlog.test	2010-12-29 05:22:52 +0000
> +++ b/mysql-test/t/mysqlbinlog.test	2011-03-04 15:28:30 +0000
> @@ -526,4 +526,32 @@ exec $MYSQL_BINLOG $MYSQLD_DATADIR/$mast
>  
>  let $binlog_file= query_get_value(SHOW MASTER STATUS, File, 1);
>  source include/show_binlog_events.inc;
> +USE test;
> +let $binlog_file=;
>  
> +
> +--echo #
> +--echo # Bug#11765416 58381: FAILED DROP DATABASE CAN BREAK STATEMENT
> +--echo #              BASED REPLICATION
> +--echo #
> +
> +--disable_warnings
> +DROP DATABASE IF EXISTS db1;
> +DROP TABLE IF EXISTS t3;
> +--enable_warnings
> +
> +CREATE DATABASE db1;
> +CREATE TABLE db1.t1 (a INT);
> +CREATE TABLE db1.t2 (b INT, KEY(b)) engine=innodb;
> +CREATE TABLE t3 (a INT, KEY (a), FOREIGN KEY(a) REFERENCES db1.t2(b))
> +  engine=innodb;
> +RESET MASTER;
> +
> +--error ER_ROW_IS_REFERENCED
> +DROP DATABASE db1;                      # Fails because of the fk
> +SHOW TABLES FROM db1;                   # t1 was dropped, t2 remains
> +--source include/show_binlog_events.inc # Check that the binlog drops t1
> +
> +# Cleanup
> +DROP TABLE t3;
> +DROP DATABASE db1;

I don't think that mysqlbinlog.test is the most appropriate place for this
test case, after all its main purpose seems to be testing of mysqlbinlog
program. In my opinion, binlog.binlog_database test and corresponding to
it extra/binlog_tests/database.test seems to be more appropriate place for
this test.

Otherwise, I am OK with this patch and think that it can be pushed into 5.5.

Also, I think that we should try to come up with a separate patch for
mysql-trunk which will remove redundancies between code responsible for
logging in mysql_rm_db() and mysql_rm_tables_no_lock().

Please check with Joro if this bug worth fixing in 5.1 (taking into account
that to do this we are likely will have to do more than a simple backport of
this patch).

-- 
Dmitry Lenev, Software Developer
Oracle Development SPB/MySQL, www.mysql.com

Are you MySQL certified?  http://www.mysql.com/certification