Nice work Li-Bing!
Patch is approved.
Best wishes,
Mats Kindahl
On 10/21/2010 05:12 AM, Li-Bing.Song@stripped wrote:
> #At file:///home/anders/Work/bzrwork/wt1/mysql-5.1-bugteam/ based on
> revid:luis.soares@stripped
>
> 3530 Li-Bing.Song@stripped 2010-10-21
> Bug#27606 GRANT statement should be replicated with DEFINER information
>
> "Grantor" columns' data is lost when replicating mysql.tables_priv.
> Slave SQL thread used its default user ''@'' as the grantor of GRANT|REVOKE
> statements executing on it.
>
> In this patch, current user is put in query log event for all GRANT and REVOKE
> statement, SQL thread uses the user in query log event as grantor.
> @ mysql-test/suite/rpl/r/rpl_do_grant.result
> Add test for this bug.
> @ mysql-test/suite/rpl/t/rpl_do_grant.test
> Add test for this bug.
> @ sql/log_event.cc
> Refactoring THD::current_user_used and related functions.
> current_user_used is used to judge if current user should be
> binlogged in query log event. So it is better to call it m_binlog_invoker.
> The related functions are renamed too.
> @ sql/sql_class.cc
> Refactoring THD::current_user_used and related functions.
> current_user_used is used to judge if current user should be
> binlogged in query log event. So it is better to call it m_binlog_invoker.
> The related functions are renamed too.
> @ sql/sql_class.h
> Refactoring THD::current_user_used and related functions.
> current_user_used is used to judge if current user should be
> binlogged in query log event. So it is better to call it m_binlog_invoker.
> The related functions are renamed too.
> @ sql/sql_parse.cc
> Call binlog_invoker() for GRANT and REVOKE statements.
>
> modified:
> mysql-test/suite/rpl/r/rpl_do_grant.result
> mysql-test/suite/rpl/t/rpl_do_grant.test
> sql/log_event.cc
> sql/sql_class.cc
> sql/sql_class.h
> sql/sql_parse.cc
> === modified file 'mysql-test/suite/rpl/r/rpl_do_grant.result'
> --- a/mysql-test/suite/rpl/r/rpl_do_grant.result 2010-05-13 15:40:31 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_do_grant.result 2010-10-21 03:12:48 +0000
> @@ -260,4 +260,27 @@ Log_name Pos Event_type Server_id End_lo
> master-bin.000001 # Query # # use `test`; grant all on *.* to foo@"1.2.3.4"
> master-bin.000001 # Query # # use `test`; revoke all privileges, grant option from
> "foo"
> DROP USER foo@"1.2.3.4";
> +
> +# Bug#27606 GRANT statement should be replicated with DEFINER information
> +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;
> +GRANT SELECT, INSERT ON mysql.user TO user_bug27606@localhost;
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +Grantor
> +root@localhost
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +Grantor
> +root@localhost
> +REVOKE SELECT ON mysql.user FROM user_bug27606@localhost;
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +Grantor
> +root@localhost
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +Grantor
> +root@localhost
> +DROP USER user_bug27606@localhost;
> "End of test"
>
> === modified file 'mysql-test/suite/rpl/t/rpl_do_grant.test'
> --- a/mysql-test/suite/rpl/t/rpl_do_grant.test 2010-05-13 15:40:31 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_do_grant.test 2010-10-21 03:12:48 +0000
> @@ -355,4 +355,25 @@ revoke all privileges, grant option from
> DROP USER foo@"1.2.3.4";
> -- sync_slave_with_master
>
> +--echo
> +--echo # Bug#27606 GRANT statement should be replicated with DEFINER information
> +--connection master
> +--source include/master-slave-reset.inc
> +--connection master
> +GRANT SELECT, INSERT ON mysql.user TO user_bug27606@localhost;
> +
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +sync_slave_with_master;
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +
> +--connection master
> +REVOKE SELECT ON mysql.user FROM user_bug27606@localhost;
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +sync_slave_with_master;
> +SELECT Grantor FROM mysql.tables_priv WHERE User='user_bug27606';
> +
> +--connection master
> +DROP USER user_bug27606@localhost;
> +
> +--source include/master-slave-end.inc
>
You could extend this test with another user to check that the
GRANT/REVOKE user is truly replicated.
> --echo "End of test"
>
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc 2010-10-13 07:25:43 +0000
> +++ b/sql/log_event.cc 2010-10-21 03:12:48 +0000
> @@ -2314,7 +2314,7 @@ bool Query_log_event::write(IO_CACHE* fi
> start+= 4;
> }
>
> - if (thd && thd->is_current_user_used())
> + if (thd && thd->need_binlog_invoker())
> {
> LEX_STRING user;
> LEX_STRING host;
>
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc 2010-09-01 17:38:34 +0000
> +++ b/sql/sql_class.cc 2010-10-21 03:12:48 +0000
> @@ -738,7 +738,7 @@ THD::THD()
> thr_lock_owner_init(&main_lock_id, &lock_info);
>
> m_internal_handler= NULL;
> - current_user_used= FALSE;
> + m_binlog_invoker= FALSE;
> memset(&invoker_user, 0, sizeof(invoker_user));
> memset(&invoker_host, 0, sizeof(invoker_host));
> }
> @@ -1247,7 +1247,7 @@ void THD::cleanup_after_query()
> where= THD::DEFAULT_WHERE;
> /* reset table map for multi-table update */
> table_map_for_update= 0;
> - clean_current_user_used();
> + m_binlog_invoker= FALSE;
> }
>
>
> @@ -3281,7 +3281,7 @@ void THD::set_query(char *query_arg, uin
>
> void THD::get_definer(LEX_USER *definer)
> {
> - set_current_user_used();
> + binlog_invoker();
> #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
> if (slave_thread && has_invoker())
> {
>
> === modified file 'sql/sql_class.h'
> --- a/sql/sql_class.h 2010-08-18 04:56:06 +0000
> +++ b/sql/sql_class.h 2010-10-21 03:12:48 +0000
> @@ -2344,9 +2344,8 @@ public:
> Protected with LOCK_thd_data mutex.
> */
> void set_query(char *query_arg, uint32 query_length_arg);
> - void set_current_user_used() { current_user_used= TRUE; }
> - bool is_current_user_used() { return current_user_used; }
> - void clean_current_user_used() { current_user_used= FALSE; }
> + void binlog_invoker() { m_binlog_invoker= TRUE; }
>
The name binlog_invoker hints that the binlog invoker (whatever that is)
is returned. Better to use a verb in the function name, like
set_binlog_invoker() or request_binlog_invoker().
> + bool need_binlog_invoker() { return m_binlog_invoker; }
> void get_definer(LEX_USER *definer);
> void set_invoker(const LEX_STRING *user, const LEX_STRING *host)
> {
> @@ -2384,7 +2383,7 @@ private:
> Current user will be binlogged into Query_log_event if current_user_used
> is TRUE; It will be stored into invoker_host and invoker_user by SQL thread.
> */
> - bool current_user_used;
> + bool m_binlog_invoker;
>
> /**
> It points to the invoker in the Query_log_event.
>
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc 2010-08-18 04:56:06 +0000
> +++ b/sql/sql_parse.cc 2010-10-21 03:12:48 +0000
> @@ -3913,6 +3913,10 @@ end_with_restore_list:
> if (check_access(thd, UPDATE_ACL, "mysql", 0, 1, 1, 0) &&
> check_global_access(thd,CREATE_USER_ACL))
> break;
> +
> + /* Replicate current user as grantor */
> + thd->binlog_invoker();
> +
> /* Conditionally writes to binlog */
> if (!(res = mysql_revoke_all(thd, lex->users_list)))
> my_ok(thd);
> @@ -3933,6 +3937,9 @@ end_with_restore_list:
> is_schema_db(select_lex->db) : 0))
> goto error;
>
> + /* Replicate current user as grantor */
> + thd->binlog_invoker();
> +
> if (thd->security_ctx->user) // If not replication
> {
> LEX_USER *user, *tmp_user;
>
>
>
>
>
>
