Hi Libing,
Find some comments in-line.
Cheers.
Li-Bing.Song@stripped wrote:
> #At file:///home/anders/work/bzrwork/bug45574/mysql-5.1-bugteam/ based on
> revid:davi.arnaut@stripped
>
> 2941 Li-Bing.Song@stripped 2009-07-28
> BUG#45574 CREATE IF NOT EXISTS is not binlogged if the object exists
>
> There is an inconsistency with DROP DATABASE|TABLE IF EXISTS and CREATE
> EVENT IF NOT EXISTS:DROP IF EXISTS statements are binlogged even if the DB or
> TABLE does not
> exist, CREATE EVENT IF NOT EXISTS are binlogged even if the event exists.
> whereas CREATE
> DATEBASE|TABLE IF NOT EXIST dose not binlog when the DB or TABLE exists.
>
> This patch includes 2 tests to test all CREATE IF NOT EXISTS statement.
> CREATE DATABASE IF NOT EXISTS,
> CREATE TABLE IF NOT EXISTS,
> CREATE TABLE IF NOT EXISTS ... LIKE have this bug.
> CREAET TABLE IF NOT EXISTS ... SELECT has this bug in row mode.
> This patch adds code to fix all above.
>
> added:
> mysql-test/suite/rpl/r/rpl_create_if_not_exists.result
> mysql-test/suite/rpl/r/rpl_create_tmp_table_if_not_exists.result
> mysql-test/suite/rpl/t/rpl_create_if_not_exists.test
> mysql-test/suite/rpl/t/rpl_create_tmp_table_if_not_exists.test
> modified:
> sql/sql_db.cc
> sql/sql_insert.cc
> sql/sql_table.cc
> === added file 'mysql-test/suite/rpl/r/rpl_create_if_not_exists.result'
> --- a/mysql-test/suite/rpl/r/rpl_create_if_not_exists.result 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_create_if_not_exists.result 2009-07-28 08:19:57
> +0000
> @@ -0,0 +1,33 @@
> +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;
> +DROP DATABASE IF EXISTS mysqltest;
> +CREATE DATABASE IF NOT EXISTS mysqltest;
> +USE mysqltest;
> +CREATE TABLE IF NOT EXISTS t(c1 int);
> +CREATE TABLE IF NOT EXISTS t1 LIKE t;
> +CREATE TABLE IF NOT EXISTS t2 SELECT * FROM t;
> +CREATE EVENT IF NOT EXISTS e
> +ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 HOUR
> +DO SELECT now();
> +DROP DATABASE mysqltest;
> +CREATE DATABASE IF NOT EXISTS mysqltest;
> +USE mysqltest;
> +CREATE TABLE IF NOT EXISTS t(c1 int);
> +CREATE TABLE IF NOT EXISTS t1 LIKE t;
> +CREATE TABLE IF NOT EXISTS t2 SELECT * FROM t;
> +CREATE EVENT IF NOT EXISTS e
> +ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 HOUR
> +DO SELECT now();
> +SHOW TABLES in mysqltest;
> +Tables_in_mysqltest
> +t
> +t1
> +t2
> +SHOW EVENTS in mysqltest;
> +Db Name Definer Time zone Type Execute at Interval value Interval
> field Starts Ends Status Originator character_set_client collation_connection Database
> Collation
> +mysqltest e @ SYSTEM ONE
> TIME - NULL NULL NULL NULL SLAVESIDE_DISABLED 1 latin1 latin1_swedish_ci latin1_swedish_ci
> +DROP DATABASE IF EXISTS mysqltest;
>
> === added file 'mysql-test/suite/rpl/r/rpl_create_tmp_table_if_not_exists.result'
> --- a/mysql-test/suite/rpl/r/rpl_create_tmp_table_if_not_exists.result 1970-01-01
> 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_create_tmp_table_if_not_exists.result 2009-07-28
> 08:19:57 +0000
> @@ -0,0 +1,22 @@
> +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;
> +DROP DATABASE IF EXISTS mysqltest;
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp(c1 int);
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp(c1 int);
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp1 LIKE tmp;
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp1 LIKE tmp;
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp2 SELECT * FROM tmp;
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp2 SELECT * FROM tmp;
> +show binlog events from <binlog_start>;
> +Log_name Pos Event_type Server_id End_log_pos Info
> +master-bin.000001 # Query # # DROP DATABASE IF EXISTS mysqltest
> +master-bin.000001 # Query # # use `test`; CREATE TEMPORARY TABLE IF NOT EXISTS
> tmp(c1 int)
> +master-bin.000001 # Query # # use `test`; CREATE TEMPORARY TABLE IF NOT EXISTS
> tmp(c1 int)
> +master-bin.000001 # Query # # use `test`; CREATE TEMPORARY TABLE IF NOT EXISTS tmp1
> LIKE tmp
> +master-bin.000001 # Query # # use `test`; CREATE TEMPORARY TABLE IF NOT EXISTS tmp1
> LIKE tmp
> +master-bin.000001 # Query # # use `test`; CREATE TEMPORARY TABLE IF NOT EXISTS tmp2
> SELECT * FROM tmp
> +master-bin.000001 # Query # # use `test`; CREATE TEMPORARY TABLE IF NOT EXISTS tmp2
> SELECT * FROM tmp
>
> === added file 'mysql-test/suite/rpl/t/rpl_create_if_not_exists.test'
> --- a/mysql-test/suite/rpl/t/rpl_create_if_not_exists.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_create_if_not_exists.test 2009-07-28 08:19:57 +0000
> @@ -0,0 +1,76 @@
> +# BUG#45574:
> +# SP: CREATE DATABASE|TABLE IF NOT EXISTS not binlogged if routine exists.
>
> +#
> +# There is an inconsistency with DROP DATABASE|TABLE IF EXISTS and CREATE
> +# EVENT IF NOT EXISTS :DROP IF EXISTS statements are binlogged even if the
> +# DB or TABLE does not exist, CREATE EVENT IF NOT EXISTS are binlogged even
> +# if the event exists. whereas CREATE DATEBASE|TABLE IF NOT EXISTS dose not
> +# binlog when the DB or TABLE exists.
>
There is an inconsistency with DROP DATABASE|TABLE|EVENT IF EXISTS and CREATE
EVENT IF NOT EXISTS: DROP IF EXISTS statements are binlogged even if either the
DB, TABLE or event does not exist; CREATE EVENT IF NOT EXISTS is binlogged even
if the event exists. In contrast, CREATE DATEBASE|TABLE IF NOT EXISTS are not
binlogged when either the DB or TABLE exists.
> +#
> +# This problem caused some of the tests fails randomly on PB or PB2.
> +#
>
This problem caused some of the tests to fail randomly on PB or PB2.
> +# Description:
> +# Fixed this bug by adding calls to write_bin_log in:
> +# mysql_create_db
> +# mysql_create_table_no_lock
> +# mysql_create_like_table
> +# create_table_from_items
> +#
> +# Test is implemented as follows:
> +# i) test each "CREATE IF NOT EXISTS" (DDL), found in MySQL 5.1 manual
> +# exclude CREATE TEMPORARY TABLE, on existent objects;
> +#
> +# Note:
> +# rpl_create_tmp_table_if_not_exists.test tests CREATE TEMPORARY TABLE cases.
> +#
> +# References:
> +# http://dev.mysql.com/doc/refman/5.1/en/sql-syntax-data-definition.html
> +#
> +
> +source include/master-slave.inc;
> +disable_warnings;
> +DROP DATABASE IF EXISTS mysqltest;
> +
> +CREATE DATABASE IF NOT EXISTS mysqltest;
> +USE mysqltest;
> +CREATE TABLE IF NOT EXISTS t(c1 int);
> +CREATE TABLE IF NOT EXISTS t1 LIKE t;
> +CREATE TABLE IF NOT EXISTS t2 SELECT * FROM t;
> +CREATE EVENT IF NOT EXISTS e
> +ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 HOUR
> +DO SELECT now();
>
> +save_master_pos;
> +
> +connection slave;
> +sync_with_master;
>
replace this by sync_slave_with_master;
> +
> +#DROP database from slave.
> +#The database and all tables can be recreated in slave
> +#if binlog of the second CREATE command is recorded and sent from master to slave.
> +DROP DATABASE mysqltest;
> +
> +connection master;
> +CREATE DATABASE IF NOT EXISTS mysqltest;
> +USE mysqltest;
> +CREATE TABLE IF NOT EXISTS t(c1 int);
> +CREATE TABLE IF NOT EXISTS t1 LIKE t;
> +CREATE TABLE IF NOT EXISTS t2 SELECT * FROM t;
> +CREATE EVENT IF NOT EXISTS e
> +ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 HOUR
> +DO SELECT now();
>
> +save_master_pos;
> +
> +connection slave;
> +sync_with_master;
>
See comment above;
> +SHOW TABLES in mysqltest;
> +#Executing time changes each test. this test do't care the executing time.
> +#It is replaced by '-'.
> +replace_column 6 -;
> +SHOW EVENTS in mysqltest;
> +
> +
> +connection master;
> +DROP DATABASE IF EXISTS mysqltest;
>
> +save_master_pos;
> +sync_slave_with_master;
>
See comment above;
> +source include/master-slave-end.inc;
>
> === added file 'mysql-test/suite/rpl/t/rpl_create_tmp_table_if_not_exists.test'
> --- a/mysql-test/suite/rpl/t/rpl_create_tmp_table_if_not_exists.test 1970-01-01
> 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_create_tmp_table_if_not_exists.test 2009-07-28
> 08:19:57 +0000
> @@ -0,0 +1,32 @@
> +# BUG#45574:
>
Copy the explanation from the other test case.
> +#
> +# Test is implemented as follows:
> +#
> +# i) test each "CREATE TEMPORARY TABLE IF EXISTS" (DDL), found in MySQL
> +# 5.1 manual, on existent objects;
> +# ii) show binlog events;
> +#
> +# Note:
> +# rpl_create_if_not_exists.test tests other cases.
> +#
> +# References:
> +# http://dev.mysql.com/doc/refman/5.1/en/sql-syntax-data-definition.html
> +#
> +
>
> +source include/master-slave.inc;
> +#CREATE TEMPORARY TABLE statements are not binlogged in row mode,
> +#So it must be test by itself.
> +source include/have_binlog_format_mixed_or_statement.inc;
> +disable_warnings;
>
> +
> +DROP DATABASE IF EXISTS mysqltest;
> +
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp(c1 int);
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp(c1 int);
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp1 LIKE tmp;
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp1 LIKE tmp;
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp2 SELECT * FROM tmp;
> +CREATE TEMPORARY TABLE IF NOT EXISTS tmp2 SELECT * FROM tmp;
> +source include/show_binlog_events.inc;
> +
> +source include/master-slave-end.inc;
>
> === modified file 'sql/sql_db.cc'
> --- a/sql/sql_db.cc 2009-05-30 13:32:28 +0000
> +++ b/sql/sql_db.cc 2009-07-28 08:19:57 +0000
> @@ -658,10 +658,8 @@ int mysql_create_db(THD *thd, char *db,
> }
> push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
> ER_DB_CREATE_EXISTS, ER(ER_DB_CREATE_EXISTS), db);
> - if (!silent)
> - my_ok(thd);
> error= 0;
> - goto exit;
> + goto not_silent;
> }
> else
> {
> @@ -698,7 +696,8 @@ int mysql_create_db(THD *thd, char *db,
> happened. (This is a very unlikely senario)
> */
> }
> -
> +
> +not_silent:
> if (!silent)
> {
> char *query;
>
> === modified file 'sql/sql_insert.cc'
> --- a/sql/sql_insert.cc 2009-06-04 23:30:08 +0000
> +++ b/sql/sql_insert.cc 2009-07-28 08:19:57 +0000
> @@ -3376,6 +3376,8 @@ static TABLE *create_table_from_items(TH
> push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
> ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
> create_table->table_name);
> + hooks->prelock(&(create_table->table), 1);
> + hooks->postlock(&(create_table->table), 1);
> DBUG_RETURN(create_table->table);
> }
>
I am not sure if this is the right approach.
What part of you test case covers this?
>
> @@ -3556,7 +3558,8 @@ select_create::prepare(List<Item> &value
> TABLE const *const table = *tables;
> if (thd->current_stmt_binlog_row_based &&
> !table->s->tmp_table &&
> - !ptr->get_create_info()->table_existed)
> + (!ptr->get_create_info()->table_existed ||
> + (ptr->get_create_info()->options &
> HA_LEX_CREATE_IF_NOT_EXISTS)))
> {
> ptr->binlog_show_create_table(tables, count);
> }
>
ok.
> === modified file 'sql/sql_table.cc'
> --- a/sql/sql_table.cc 2009-06-02 09:07:17 +0000
> +++ b/sql/sql_table.cc 2009-07-28 08:19:57 +0000
> @@ -3432,6 +3432,41 @@ void sp_prepare_create_field(THD *thd, C
>
>
> /*
> + Write CREATE TABLE binlog
> +
> + SYNOPSIS
> + write_create_table_bin_log()
> + thd Thread object
> + create_info Create information
> + internal_tmp_table Set to 1 if this is an internal temporary table
> +
> + DESCRIPTION
> + This function only is called in mysql_create_table_no_lock and
> + mysql_create_table
> +
> + RETURN VALUES
> + NONE
> + */
> +static inline void write_create_table_bin_log(THD *thd,
> + const HA_CREATE_INFO *create_info,
> + bool internal_tmp_table)
> +{
> + /*
> + Don't write statement if:
> + - It is an internal temporary table,
> + - Row-based logging is used and it we are creating a temporary table, or
> + - The binary log is not open.
> + Otherwise, the statement shall be binlogged.
> + */
> + if (!internal_tmp_table &&
> + (!thd->current_stmt_binlog_row_based ||
> + (thd->current_stmt_binlog_row_based &&
> + !(create_info->options & HA_LEX_CREATE_TMP_TABLE))))
> + write_bin_log(thd, TRUE, thd->query, thd->query_length);
> +}
> +
> +
> +/*
> Create a table
>
> SYNOPSIS
> @@ -3694,6 +3729,7 @@ bool mysql_create_table_no_lock(THD *thd
> ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
> alias);
> error= 0;
>
Do you think it is possible to re-organize the code in order to avoid
the function above and keep just one call to write_bin_log in the
mysql_create_table_no_lock?
> + write_create_table_bin_log(thd, create_info, internal_tmp_table);
> goto err;
>
ok.
> }
> my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alias);
> @@ -3814,18 +3850,7 @@ bool mysql_create_table_no_lock(THD *thd
> thd->thread_specific_used= TRUE;
> }
>
> - /*
> - Don't write statement if:
> - - It is an internal temporary table,
> - - Row-based logging is used and it we are creating a temporary table, or
> - - The binary log is not open.
> - Otherwise, the statement shall be binlogged.
> - */
> - if (!internal_tmp_table &&
> - (!thd->current_stmt_binlog_row_based ||
> - (thd->current_stmt_binlog_row_based &&
> - !(create_info->options & HA_LEX_CREATE_TMP_TABLE))))
> - write_bin_log(thd, TRUE, thd->query, thd->query_length);
> + write_create_table_bin_log(thd, create_info, internal_tmp_table);
> error= FALSE;
> unlock_and_end:
> VOID(pthread_mutex_unlock(&LOCK_open));
> @@ -3841,6 +3866,7 @@ warn:
> ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
> alias);
> create_info->table_existed= 1; // Mark that table existed
> + write_create_table_bin_log(thd, create_info, internal_tmp_table);
> goto unlock_and_end;
> }
>
>
ok.
> @@ -3892,6 +3918,7 @@ bool mysql_create_table(THD *thd, const
> table_name);
> create_info->table_existed= 1;
> result= FALSE;
> + write_create_table_bin_log(thd, create_info, internal_tmp_table);
> }
> else
> {
> @@ -5231,6 +5258,7 @@ bool mysql_create_like_table(THD* thd, T
> goto err; /* purecov: inspected */
> }
>
Why do you need to call it from here? Note that the function
mysql_create_table_no_lock will still be called. Shouldn't it
be responsible for calling write_create_table_bin_log in this case?
Is there a corner case? If so, add some comments.
>
> +binlog:
> DBUG_EXECUTE_IF("sleep_create_like_before_binlogging", my_sleep(6000000););
>
> /*
> @@ -5305,6 +5333,7 @@ table_exists:
> push_warning(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
> ER_TABLE_EXISTS_ERROR,warn_buff);
> res= FALSE;
> + goto binlog;
> }
> else
> my_error(ER_TABLE_EXISTS_ERROR, MYF(0), table_name);
>
>
Please, move this part of code to a place before the label binlog
and change whatever is necessary in the logic to do that.
> ------------------------------------------------------------------------
>
>
>
