Hi Dao-Gang,
Make sure that you run MTR before pushing the patch.
There are some test cases that are failing: See one more
comment in-line.
Cheers,
Alfranio Correia wrote:
> 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);
>>
Maybe it would be better to replace definer_exist by if_definer_exists.
>> + res= Events::create_event(thd, lex->event_parse_data, if_not_exists,
> definer_exist);
>>
>> break;
>> }
>> case SQLCOM_ALTER_EVENT:
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>>
>>
>
>
>
