Hi Libing,
Great work.
Please, before pushing the patch, change the commit message and the
comment in the test case, specifically,
replace "BUG #46572 DROP temp-table IF EXISTS does not have a consistent
behavior in ROW mode" by
"BUG #46572 DROP TEMPORARY table IF EXISTS does not have a consistent behavior in ROW
mode"
I've updated the title of the bug.
Cheers.
Li-Bing.Song@stripped wrote:
> #At file:///home/anders/work/bzrwork/July/bug45574/mysql-5.1-bugteam/ based on
> revid:joro@stripped
>
> 3117 Li-Bing.Song@stripped 2009-09-23
> BUG #46572 DROP temp-table IF EXISTS does not have a consistent behavior in ROW
> mode
>
> In RBR, 'DROP TEMPORARY TABLE IF EXISTS...' statement is binlogged when the
> table
> does not exist.
>
> In fact, 'DROP TEMPORARY TABLE ...' statement should never be binlogged in RBR
> no matter if the table exists or not.
> This patch addresses this by checking whether we are dropping a
> temporary table or not, when building the custom drop statement.
>
> modified:
> mysql-test/extra/binlog_tests/drop_temp_table.test
> mysql-test/suite/binlog/r/binlog_row_drop_tmp_tbl.result
> mysql-test/suite/binlog/r/binlog_stm_drop_tmp_tbl.result
> sql/sql_table.cc
> === modified file 'mysql-test/extra/binlog_tests/drop_temp_table.test'
> --- a/mysql-test/extra/binlog_tests/drop_temp_table.test 2007-06-15 16:56:11 +0000
> +++ b/mysql-test/extra/binlog_tests/drop_temp_table.test 2009-09-23 06:44:58 +0000
> @@ -1,27 +1,62 @@
>
> --disable_warnings
> -drop database if exists `drop-temp+table-test`;
> +DROP DATABASE IF EXISTS `drop-temp+table-test`;
> --enable_warnings
>
> connect (con1,localhost,root,,);
> connect (con2,localhost,root,,);
> connection con1;
> -reset master;
> -create database `drop-temp+table-test`;
> -use `drop-temp+table-test`;
> -create temporary table shortn1 (a int);
> -create temporary table `table:name` (a int);
> -create temporary table shortn2 (a int);
> -select get_lock("a",10);
> +RESET MASTER;
> +CREATE DATABASE `drop-temp+table-test`;
> +USE `drop-temp+table-test`;
> +CREATE TEMPORARY TABLE shortn1 (a INT);
> +CREATE TEMPORARY TABLE `table:name` (a INT);
> +CREATE TEMPORARY TABLE shortn2 (a INT);
> +
> +#############################################################################
> +# BUG#46572 DROP temp-table IF EXISTS does not have a consistent behavior in
> +# ROW mode
> +#
> +# In RBR, 'DROP TEMPORARY TABLE ...' statement should never be binlogged no
> +# matter if the tables exist or not. In contrast, both in SBR and MBR, the
> +# statement should be always binlogged no matter if the tables exist or not.
> +#############################################################################
> +CREATE TEMPORARY TABLE tmp(c1 int);
> +CREATE TEMPORARY TABLE tmp1(c1 int);
> +CREATE TEMPORARY TABLE tmp2(c1 int);
> +CREATE TEMPORARY TABLE tmp3(c1 int);
> +CREATE TABLE t(c1 int);
> +
> +DROP TEMPORARY TABLE IF EXISTS tmp;
> +
> +--disable_warnings
> +# Before fixing BUG#46572, 'DROP TEMPORARY TABLE IF EXISTS...' statement was
> +# binlogged when the table did not exist in RBR.
> +DROP TEMPORARY TABLE IF EXISTS tmp;
> +
> +# In RBR, 'DROP TEMPORARY TABLE ...' statement is never binlogged no matter if
> +# the tables exist or not.
> +DROP TEMPORARY TABLE IF EXISTS tmp, tmp1;
> +DROP TEMPORARY TABLE tmp3;
> +
> +#In RBR, tmp2 will NOT be binlogged, because it is a temporary table.
> +DROP TABLE IF EXISTS tmp2, t;
> +
> +#In RBR, tmp2 will be binlogged, because it does not exist and master do not know
> +# whether it is a temporary table or not.
> +DROP TABLE IF EXISTS tmp2, t;
> +--enable_warnings
> +
> +SELECT GET_LOCK("a",10);
> disconnect con1;
>
> connection con2;
> # We want to SHOW BINLOG EVENTS, to know what was logged. But there is no
> # guarantee that logging of the terminated con1 has been done yet.
> # To be sure that logging has been done, we use a user lock.
> -select get_lock("a",10);
> -let $VERSION=`select version()`;
> +SELECT GET_LOCK("a",10);
> +let $VERSION=`SELECT VERSION()`;
> source include/show_binlog_events.inc;
> -drop database `drop-temp+table-test`;
> +DROP DATABASE `drop-temp+table-test`;
>
> # End of 4.1 tests
>
> === modified file 'mysql-test/suite/binlog/r/binlog_row_drop_tmp_tbl.result'
> --- a/mysql-test/suite/binlog/r/binlog_row_drop_tmp_tbl.result 2007-06-27 12:28:02
> +0000
> +++ b/mysql-test/suite/binlog/r/binlog_row_drop_tmp_tbl.result 2009-09-23 06:44:58
> +0000
> @@ -1,17 +1,31 @@
> -drop database if exists `drop-temp+table-test`;
> -reset master;
> -create database `drop-temp+table-test`;
> -use `drop-temp+table-test`;
> -create temporary table shortn1 (a int);
> -create temporary table `table:name` (a int);
> -create temporary table shortn2 (a int);
> -select get_lock("a",10);
> -get_lock("a",10)
> +DROP DATABASE IF EXISTS `drop-temp+table-test`;
> +RESET MASTER;
> +CREATE DATABASE `drop-temp+table-test`;
> +USE `drop-temp+table-test`;
> +CREATE TEMPORARY TABLE shortn1 (a INT);
> +CREATE TEMPORARY TABLE `table:name` (a INT);
> +CREATE TEMPORARY TABLE shortn2 (a INT);
> +CREATE TEMPORARY TABLE tmp(c1 int);
> +CREATE TEMPORARY TABLE tmp1(c1 int);
> +CREATE TEMPORARY TABLE tmp2(c1 int);
> +CREATE TEMPORARY TABLE tmp3(c1 int);
> +CREATE TABLE t(c1 int);
> +DROP TEMPORARY TABLE IF EXISTS tmp;
> +DROP TEMPORARY TABLE IF EXISTS tmp;
> +DROP TEMPORARY TABLE IF EXISTS tmp, tmp1;
> +DROP TEMPORARY TABLE tmp3;
> +DROP TABLE IF EXISTS tmp2, t;
> +DROP TABLE IF EXISTS tmp2, t;
> +SELECT GET_LOCK("a",10);
> +GET_LOCK("a",10)
> 1
> -select get_lock("a",10);
> -get_lock("a",10)
> +SELECT GET_LOCK("a",10);
> +GET_LOCK("a",10)
> 1
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # create database `drop-temp+table-test`
> -drop database `drop-temp+table-test`;
> +master-bin.000001 # Query # # CREATE DATABASE `drop-temp+table-test`
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TABLE t(c1 int)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TABLE IF EXISTS `t`
> /* generated by server */
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TABLE IF EXISTS tmp2,
> t
> +DROP DATABASE `drop-temp+table-test`;
>
> === modified file 'mysql-test/suite/binlog/r/binlog_stm_drop_tmp_tbl.result'
> --- a/mysql-test/suite/binlog/r/binlog_stm_drop_tmp_tbl.result 2009-08-28 09:45:57
> +0000
> +++ b/mysql-test/suite/binlog/r/binlog_stm_drop_tmp_tbl.result 2009-09-23 06:44:58
> +0000
> @@ -1,21 +1,43 @@
> -drop database if exists `drop-temp+table-test`;
> -reset master;
> -create database `drop-temp+table-test`;
> -use `drop-temp+table-test`;
> -create temporary table shortn1 (a int);
> -create temporary table `table:name` (a int);
> -create temporary table shortn2 (a int);
> -select get_lock("a",10);
> -get_lock("a",10)
> +DROP DATABASE IF EXISTS `drop-temp+table-test`;
> +RESET MASTER;
> +CREATE DATABASE `drop-temp+table-test`;
> +USE `drop-temp+table-test`;
> +CREATE TEMPORARY TABLE shortn1 (a INT);
> +CREATE TEMPORARY TABLE `table:name` (a INT);
> +CREATE TEMPORARY TABLE shortn2 (a INT);
> +CREATE TEMPORARY TABLE tmp(c1 int);
> +CREATE TEMPORARY TABLE tmp1(c1 int);
> +CREATE TEMPORARY TABLE tmp2(c1 int);
> +CREATE TEMPORARY TABLE tmp3(c1 int);
> +CREATE TABLE t(c1 int);
> +DROP TEMPORARY TABLE IF EXISTS tmp;
> +DROP TEMPORARY TABLE IF EXISTS tmp;
> +DROP TEMPORARY TABLE IF EXISTS tmp, tmp1;
> +DROP TEMPORARY TABLE tmp3;
> +DROP TABLE IF EXISTS tmp2, t;
> +DROP TABLE IF EXISTS tmp2, t;
> +SELECT GET_LOCK("a",10);
> +GET_LOCK("a",10)
> 1
> -select get_lock("a",10);
> -get_lock("a",10)
> +SELECT GET_LOCK("a",10);
> +GET_LOCK("a",10)
> 1
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # create database `drop-temp+table-test`
> -master-bin.000001 # Query # # use `drop-temp+table-test`; create temporary table
> shortn1 (a int)
> -master-bin.000001 # Query # # use `drop-temp+table-test`; create temporary table
> `table:name` (a int)
> -master-bin.000001 # Query # # use `drop-temp+table-test`; create temporary table
> shortn2 (a int)
> +master-bin.000001 # Query # # CREATE DATABASE `drop-temp+table-test`
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TEMPORARY TABLE
> shortn1 (a INT)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TEMPORARY TABLE
> `table:name` (a INT)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TEMPORARY TABLE
> shortn2 (a INT)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TEMPORARY TABLE
> tmp(c1 int)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TEMPORARY TABLE
> tmp1(c1 int)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TEMPORARY TABLE
> tmp2(c1 int)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TEMPORARY TABLE
> tmp3(c1 int)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; CREATE TABLE t(c1 int)
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TEMPORARY TABLE IF
> EXISTS tmp
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TEMPORARY TABLE IF
> EXISTS tmp
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TEMPORARY TABLE IF
> EXISTS tmp, tmp1
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TEMPORARY TABLE tmp3
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TABLE IF EXISTS tmp2,
> t
> +master-bin.000001 # Query # # use `drop-temp+table-test`; DROP TABLE IF EXISTS tmp2,
> t
> master-bin.000001 # Query # # use `drop-temp+table-test`; DROP /*!40005 TEMPORARY */
> TABLE IF EXISTS `shortn2`,`table:name`,`shortn1`
> -drop database `drop-temp+table-test`;
> +DROP DATABASE `drop-temp+table-test`;
>
> === modified file 'sql/sql_table.cc'
> --- a/sql/sql_table.cc 2009-09-18 13:01:18 +0000
> +++ b/sql/sql_table.cc 2009-09-23 06:44:58 +0000
> @@ -1949,7 +1949,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
> being built. The string always end in a comma and the comma
> will be chopped off before being written to the binary log.
> */
> - if (thd->current_stmt_binlog_row_based && !dont_log_query)
> + if (!drop_temporary && thd->current_stmt_binlog_row_based &&
> !dont_log_query)
> {
> non_temp_tables_count++;
> /*
>
>
> ------------------------------------------------------------------------
>
>
>