Hi Dao-Gang,
Nice work.
Find some comments in-line.
Cheers.
Dao-Gang.Qu@stripped wrote:
> #At file:///home/daogangq/mysql/bzrwork/bug44331/5.1-bugteam/ based on
> revid:joro@stripped
>
> 2994 Dao-Gang.Qu@stripped 2009-07-10
> Bug #44331 Restore of database with events produces warning in replication
>
I think the description of the problem is wrong. And assuming that
I did not understand the problem the description of the solution does
not seem to fix the problem ;).
>
> The real problem is that If the user create EVENT without DEFINER clause set
> explicitly,
> the CURRENT_USER is set to DEFINER in current thread. The CURRENT_USER is
> mysqld startup
> user on master, but the CURRENT_USER is empty on slave for replication. So the
> definer
> is not coincident between master and slave.
>
>
>
The description below is fine, although there are some English typos.
> For fixing the problem, the CURRENT_USER will be written into the binary log as
>
> default definer for replication on master, If the user create EVENT without
> DEFINER clause set explicitly.
>
> @ mysql-test/suite/rpl/r/rpl_restore_event_warning.result
> Add test result for bug#44331
> @ mysql-test/suite/rpl/t/rpl_restore_event_warning.test
> Add test file for bug#44331
> @ sql/events.cc
> The "create_query_string(...)" is added for generating the CREATE query
> string
> from the thd information.
> Added a new parameter "definer_exist" to 'Events::create_event(...)' function
> for
> handling the case that the user create EVENT without DEFINER clause set
> explicitly.
> the CURRENT_USER will be written into the binary log as default definer for
> replication.
> @ sql/events.h
> Added a new parameter "definer_exist" with default actual parameter to
> 'Events::create_event(...)' function declaration.
> @ sql/sql_parse.cc
> Add a new local variable "definer_exist" to "mysql_execute_command(THD *thd)"
> function
> to indicate if the definer has been specified by user.
>
> added:
> mysql-test/suite/rpl/r/rpl_restore_event_warning.result
> mysql-test/suite/rpl/t/rpl_restore_event_warning.test
> modified:
> sql/events.cc
> sql/events.h
> sql/sql_parse.cc
> === added file 'mysql-test/suite/rpl/r/rpl_restore_event_warning.result'
> --- a/mysql-test/suite/rpl/r/rpl_restore_event_warning.result 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_restore_event_warning.result 2009-07-10 08:47:20
> +0000
> @@ -0,0 +1,19 @@
> +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 mysqltest;
> +CREATE TABLE mysqltest.t1(details CHAR(30));
> +CREATE EVENT mysqltest.e1 ON SCHEDULE AT CURRENT_TIMESTAMP
> +ON COMPLETION PRESERVE DISABLE
> +DO INSERT INTO mysqltest.t1 VALUES('event e1 fired');
> +select EVENT_SCHEMA, EVENT_NAME, DEFINER from information_schema.events;
> +EVENT_SCHEMA EVENT_NAME DEFINER
> +mysqltest e1 root@localhost
> +select EVENT_SCHEMA, EVENT_NAME, DEFINER from information_schema.events;
> +EVENT_SCHEMA EVENT_NAME DEFINER
> +mysqltest e1 root@localhost
> +drop database mysqltest;
>
>
This is a small test can you merge it with other test?
Maybe: ./suite/rpl/t/rpl_events.test
> === added file 'mysql-test/suite/rpl/t/rpl_restore_event_warning.test'
> --- a/mysql-test/suite/rpl/t/rpl_restore_event_warning.test 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_restore_event_warning.test 2009-07-10 08:47:20
> +0000
> @@ -0,0 +1,28 @@
> +#
> +# BUG#44331
> +# This test verifies if the default definer is coincident between master and slave,
> +# when the user create EVENT without DEFINER clause set explicitly.
> +#
> +
> +source include/master-slave.inc;
> +--disable_warnings
> +drop database if exists mysqltest;
> +--enable_warnings
> +
> +connection master;
> +CREATE DATABASE mysqltest;
> +CREATE TABLE mysqltest.t1(details CHAR(30));
> +CREATE EVENT mysqltest.e1 ON SCHEDULE AT CURRENT_TIMESTAMP
> + ON COMPLETION PRESERVE DISABLE
> + DO INSERT INTO mysqltest.t1 VALUES('event e1 fired');
> +select EVENT_SCHEMA, EVENT_NAME, DEFINER from information_schema.events;
> +
> +sync_slave_with_master;
> +connection slave;
> +select EVENT_SCHEMA, EVENT_NAME, DEFINER from information_schema.events;
> +
> +connection master;
> +drop database mysqltest;
> +save_master_pos;
> +connection slave;
> +sync_with_master;
>
> === modified file 'sql/events.cc'
> --- a/sql/events.cc 2009-04-09 06:22:06 +0000
> +++ b/sql/events.cc 2009-07-10 08:47:20 +0000
> @@ -340,6 +340,41 @@ common_1_lev_code:
> return 0;
> }
>
> +/**
> + Generates the CREATE query string from the thd information.
> + Set the specified definer to the default value, which is the
> + current user in the thread.
> +
> + @param[in] thd thread handler
> + @param[in] buf query string
> + @return
> + Returns TRUE on success, FALSE on (alloc) failure.
> +*/
> +static bool
> +create_query_string(THD *thd, String *buf)
> +{
> + int userlen= strlen(thd->lex->definer->user.str);
> + int hostlen= strlen(thd->lex->definer->host.str);
> + char *indexofspace= strstr(thd->query, " ");
> +
> + /* Make some room to begin with */
> + if (buf->alloc(100 + userlen + hostlen + thd->query_length +
> + 15 /* length of " DEFINER=''@'' "*/))
>
./sql/sql_show.cc:1614:void append_definer(THD *thd, String *buffer ...
Use the function above, it seems to be default.
> + return FALSE;
> + /* Append the "CREATE" part of thd->query */
> + buf->append(thd->query, indexofspace - thd->query);
> + buf->append(STRING_WITH_LEN(" DEFINER='"));
> + /* Append the definer user */
> + buf->append(thd->lex->definer->user.str);
> + buf->append(STRING_WITH_LEN("'@'"));
> + /* Append the definer host*/
> + buf->append(thd->lex->definer->host.str);
> + buf->append(STRING_WITH_LEN("'"));
> + /* Append the left part of thd->query */
> + buf->append(indexofspace);
> +
> + return TRUE;
> +}
>
> /**
> Create a new event.
> @@ -359,7 +394,7 @@ common_1_lev_code:
>
> bool
> Events::create_event(THD *thd, Event_parse_data *parse_data,
> - bool if_not_exists)
> + bool if_not_exists, bool definer_exist)
> {
> int ret;
> DBUG_ENTER("Events::create_event");
> @@ -439,7 +474,15 @@ Events::create_event(THD *thd, Event_par
> {
> /* Binlog the create event. */
> DBUG_ASSERT(thd->query && thd->query_length);
> - write_bin_log(thd, TRUE, thd->query, thd->query_length);
> +
> + String log_query;
> + /* if the definer does not exist, CURRENT_USER will be written
> + into the binary log for replication */
> + if (!definer_exist && create_query_string(thd, &log_query))
> + write_bin_log(thd, TRUE, log_query.c_ptr(), log_query.length());
> + else
> + write_bin_log(thd, TRUE, thd->query, thd->query_length);
> +
> }
> }
> pthread_mutex_unlock(&LOCK_event_metadata);
>
> === modified file 'sql/events.h'
> --- a/sql/events.h 2007-08-15 15:08:44 +0000
> +++ b/sql/events.h 2009-07-10 08:47:20 +0000
> @@ -107,7 +107,7 @@ public:
> switch_event_scheduler_state(enum enum_opt_event_scheduler new_state);
>
> static bool
> - create_event(THD *thd, Event_parse_data *parse_data, bool if_exists);
>
Please, don't use default values.
> + create_event(THD *thd, Event_parse_data *parse_data, bool if_exists, bool
> definer_exist= TRUE);
>
>
> static bool
> update_event(THD *thd, Event_parse_data *parse_data,
>
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc 2009-07-01 12:32:04 +0000
> +++ b/sql/sql_parse.cc 2009-07-10 08:47:20 +0000
>
If for some reason the request above is hard to address,
you can keep the signature and access the lex->definer through the THD
in the Events::create.
> @@ -3707,6 +3707,10 @@ end_with_restore_list:
> "function calls as part of this statement");
> break;
> }
> +
> + bool definer_exist= TRUE;
> + if (!lex->definer)
> + definer_exist= FALSE;
>
> res= sp_process_definer(thd);
> if (res)
> @@ -3715,9 +3719,10 @@ end_with_restore_list:
> switch (lex->sql_command) {
> case SQLCOM_CREATE_EVENT:
> {
> +
> bool if_not_exists= (lex->create_info.options &
> HA_LEX_CREATE_IF_NOT_EXISTS);
> - res= Events::create_event(thd, lex->event_parse_data, if_not_exists);
> + res= Events::create_event(thd, lex->event_parse_data, if_not_exists,
> definer_exist);
> break;
> }
> case SQLCOM_ALTER_EVENT:
>
>
> ------------------------------------------------------------------------
>
>
>
