List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:January 20 2008 11:56am
Subject:Re: bk commit into 5.0 tree (hezx:1.2565) BUG#33862
View as plain text  
On 2008-01-18 Fri 11:43 -0500,Mats Kindahl wrote:
> Hi Jason!
> 
> The patch looks good and is OK to push.
> 
> It would, however, be good to have a test case for each of CREATE, DROP, 
> and RENAME, where more than one non-existing user is processed, and see 
> that the error message hold all of them (AIUI, the error message should 
> contain all users that cannot be processed).

Good, thanks

> 
> Just my few cents,
> Mats Kindahl
> 
> hezx@stripped wrote:
> > Below is the list of changes that have just been committed into a local
> > 5.0 repository of hezx. When hezx does a push these changes will
> > be propagated to the main repository and, within 24 hours after the
> > push, to the public repository.
> > For information on how to access the public repository
> > see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
> >
> > ChangeSet@stripped, 2008-01-17 14:53:55+08:00, hezx@stripped +3 -0
> >   BUG#33862 completely failed DROP USER statement gets replicated
> >   
> >   When create/rename/drop users, the statement will be logged regardless of
> error, even if no data has been changed, the statement will be logged.
> >   
> >   When create/rename/drop users, don't write the binlog if the statement make no
> changes, if the statement do made any changes, log the statement with possible error
> code.
> >
> >   mysql-test/r/rpl_user.result@stripped, 2008-01-17 14:53:51+08:00,
> hezx@stripped +42 -0
> >     New BitKeeper file ``mysql-test/r/rpl_user.result''
> >
> >   mysql-test/r/rpl_user.result@stripped, 2008-01-17 14:53:51+08:00,
> hezx@stripped +0 -0
> >
> >   mysql-test/t/rpl_user.test@stripped, 2008-01-17 14:53:51+08:00, hezx@stripped
> +57 -0
> >     New BitKeeper file ``mysql-test/t/rpl_user.test''
> >
> >   mysql-test/t/rpl_user.test@stripped, 2008-01-17 14:53:51+08:00, hezx@stripped
> +0 -0
> >
> >   sql/sql_acl.cc@stripped, 2008-01-17 14:53:51+08:00, hezx@stripped +17 -7
> >     when create/rename/drop users, don't write the binlog if the statement make
> no changes
> >
> > diff -Nrup a/mysql-test/r/rpl_user.result b/mysql-test/r/rpl_user.result
> > --- /dev/null	Wed Dec 31 16:00:00 196900
> > +++ b/mysql-test/r/rpl_user.result	2008-01-17 14:53:51 +08:00
> > @@ -0,0 +1,42 @@
> > +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;
> > +reset master;
> > +delete from mysql.user where Host='fakehost';
> > +create user 'foo'@'fakehost';
> > +create user 'foo'@'fakehost';
> > +ERROR HY000: Operation CREATE USER failed for 'foo'@'fakehost'
> > +create user 'foo'@'fakehost', 'bar'@'fakehost';
> > +ERROR HY000: Operation CREATE USER failed for 'foo'@'fakehost'
> > +select Host,User from mysql.user where Host='fakehost';
> > +Host	User
> > +fakehost	bar
> > +fakehost	foo
> > +rename user 'foo'@'fakehost' to 'foofoo'@'fakehost';
> > +rename user 'not_exist_user'@'fakehost' to 'foobar'@'fakehost';
> > +ERROR HY000: Operation RENAME USER failed for 'not_exist_user'@'fakehost'
> > +rename user 'not_exist_user'@'fakehost' to 'foobar'@'fakehost',
> 'bar'@'fakehost' to 'barbar'@'fakehost';
> > +ERROR HY000: Operation RENAME USER failed for 'not_exist_user'@'fakehost'
> > +select Host,User from mysql.user where Host='fakehost';
> > +Host	User
> > +fakehost	barbar
> > +fakehost	foofoo
> > +drop user 'foofoo'@'fakehost';
> > +drop user 'not_exist_user'@'fakehost';
> > +ERROR HY000: Operation DROP USER failed for 'not_exist_user'@'fakehost'
> > +drop user 'not_exist_user'@'fakehost', 'barbar'@'fakehost';
> > +ERROR HY000: Operation DROP USER failed for 'not_exist_user'@'fakehost'
> > +select Host,User from mysql.user where Host='fakehost';
> > +Host	User
> > +show binlog events from 98;
> > +Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> > +master-bin.000001	98	Query	1	#	use `test`; delete from mysql.user where
> Host='fakehost'
> > +master-bin.000001	205	Query	1	#	use `test`; create user 'foo'@'fakehost'
> > +master-bin.000001	296	Query	1	#	use `test`; create user 'foo'@'fakehost',
> 'bar'@'fakehost'
> > +master-bin.000001	405	Query	1	#	use `test`; rename user 'foo'@'fakehost' to
> 'foofoo'@'fakehost'
> > +master-bin.000001	519	Query	1	#	use `test`; rename user
> 'not_exist_user'@'fakehost' to 'foobar'@'fakehost', 'bar'@'fakehost' to
> 'barbar'@'fakehost'
> > +master-bin.000001	685	Query	1	#	use `test`; drop user 'foofoo'@'fakehost'
> > +master-bin.000001	777	Query	1	#	use `test`; drop user
> 'not_exist_user'@'fakehost', 'barbar'@'fakehost'
> > diff -Nrup a/mysql-test/t/rpl_user.test b/mysql-test/t/rpl_user.test
> > --- /dev/null	Wed Dec 31 16:00:00 196900
> > +++ b/mysql-test/t/rpl_user.test	2008-01-17 14:53:51 +08:00
> > @@ -0,0 +1,57 @@
> > +--source include/master-slave.inc
> > +--source include/have_binlog_format_statement.inc
> > +
> > +set @@session.binlog_format='statement';
> > +
> > +reset master;
> > +
> > +#
> > +# remove all users will be used in the test
> > +#
> > +delete from mysql.user where Host='fakehost';
> > +sync_slave_with_master;
> > +
> > +#
> > +# Test create user
> > +#
> > +connection master;
> > +create user 'foo'@'fakehost';
> > +--error ER_CANNOT_USER
> > +create user 'foo'@'fakehost';
> > +--error ER_CANNOT_USER
> > +create user 'foo'@'fakehost', 'bar'@'fakehost';
> > +
> > +sync_slave_with_master;
> > +select Host,User from mysql.user where Host='fakehost';
> > +
> > +#
> > +# Test rename user
> > +#
> > +connection master;
> > +rename user 'foo'@'fakehost' to 'foofoo'@'fakehost';
> > +--error ER_CANNOT_USER
> > +rename user 'not_exist_user'@'fakehost' to 'foobar'@'fakehost';
> > +--error ER_CANNOT_USER
> > +rename user 'not_exist_user'@'fakehost' to 'foobar'@'fakehost',
> 'bar'@'fakehost' to 'barbar'@'fakehost';
> > +
> > +sync_slave_with_master;
> > +select Host,User from mysql.user where Host='fakehost';
> > +
> > +#
> > +# Test drop user
> > +#
> > +connection master;
> > +drop user 'foofoo'@'fakehost';
> > +--error ER_CANNOT_USER
> > +drop user 'not_exist_user'@'fakehost';
> > +--error ER_CANNOT_USER
> > +drop user 'not_exist_user'@'fakehost', 'barbar'@'fakehost';
> > +
> > +sync_slave_with_master;
> > +select Host,User from mysql.user where Host='fakehost';
> > +
> > +#
> > +# show the binlog events on the master
> > +#
> > +connection master;
> > +source include/show_binlog_events.inc;
> > diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc
> > --- a/sql/sql_acl.cc	2007-10-31 20:41:46 +08:00
> > +++ b/sql/sql_acl.cc	2008-01-17 14:53:51 +08:00
> > @@ -5333,6 +5333,7 @@ bool mysql_create_user(THD *thd, List <L
> >    LEX_USER *user_name, *tmp_user_name;
> >    List_iterator <LEX_USER> user_list(list);
> >    TABLE_LIST tables[GRANT_TABLES];
> > +  bool some_users_created= FALSE;
> >    DBUG_ENTER("mysql_create_user");
> >  
> >    /* CREATE USER may be skipped on replication client. */
> > @@ -5361,6 +5362,7 @@ bool mysql_create_user(THD *thd, List <L
> >        continue;
> >      }
> >  
> > +    some_users_created= TRUE;
> >      sql_mode= thd->variables.sql_mode;
> >      if (replace_user_table(thd, tables[0].table, *user_name, 0, 0, 1, 0))
> >      {
> > @@ -5371,7 +5373,10 @@ bool mysql_create_user(THD *thd, List <L
> >  
> >    VOID(pthread_mutex_unlock(&acl_cache->lock));
> >  
> > -  if (mysql_bin_log.is_open())
> > +  if (result)
> > +    my_error(ER_CANNOT_USER, MYF(0), "CREATE USER", wrong_users.c_ptr_safe());
> > +
> > +  if (some_users_created && mysql_bin_log.is_open())
> >    {
> >      Query_log_event qinfo(thd, thd->query, thd->query_length, 0, FALSE);
> >      mysql_bin_log.write(&qinfo);
> > @@ -5379,8 +5384,6 @@ bool mysql_create_user(THD *thd, List <L
> >  
> >    rw_unlock(&LOCK_grant);
> >    close_thread_tables(thd);
> > -  if (result)
> > -    my_error(ER_CANNOT_USER, MYF(0), "CREATE USER", wrong_users.c_ptr_safe());
> >    DBUG_RETURN(result);
> >  }
> >  
> > @@ -5405,6 +5408,7 @@ bool mysql_drop_user(THD *thd, List <LEX
> >    LEX_USER *user_name, *tmp_user_name;
> >    List_iterator <LEX_USER> user_list(list);
> >    TABLE_LIST tables[GRANT_TABLES];
> > +  bool some_users_deleted= FALSE;
> >    DBUG_ENTER("mysql_drop_user");
> >  
> >    /* DROP USER may be skipped on replication client. */
> > @@ -5426,7 +5430,9 @@ bool mysql_drop_user(THD *thd, List <LEX
> >      {
> >        append_user(&wrong_users, user_name);
> >        result= TRUE;
> > +      continue;
> >      }
> > +    some_users_deleted= TRUE;
> >    }
> >  
> >    /* Rebuild 'acl_check_hosts' since 'acl_users' has been modified */
> > @@ -5440,7 +5446,7 @@ bool mysql_drop_user(THD *thd, List <LEX
> >    DBUG_PRINT("info", ("thd->net.last_errno: %d", thd->net.last_errno));
> >    DBUG_PRINT("info", ("thd->net.last_error: %s", thd->net.last_error));
> >  
> > -  if (mysql_bin_log.is_open())
> > +  if (some_users_deleted && mysql_bin_log.is_open())
> >    {
> >      Query_log_event qinfo(thd, thd->query, thd->query_length, 0, FALSE);
> >      mysql_bin_log.write(&qinfo);
> > @@ -5473,6 +5479,7 @@ bool mysql_rename_user(THD *thd, List <L
> >    LEX_USER *user_to, *tmp_user_to;
> >    List_iterator <LEX_USER> user_list(list);
> >    TABLE_LIST tables[GRANT_TABLES];
> > +  bool some_users_renamed= FALSE;
> >    DBUG_ENTER("mysql_rename_user");
> >  
> >    /* RENAME USER may be skipped on replication client. */
> > @@ -5506,7 +5513,9 @@ bool mysql_rename_user(THD *thd, List <L
> >      {
> >        append_user(&wrong_users, user_from);
> >        result= TRUE;
> > +      continue;
> >      }
> > +    some_users_renamed= TRUE;
> >    }
> >    
> >    /* Rebuild 'acl_check_hosts' since 'acl_users' has been modified */
> > @@ -5514,7 +5523,10 @@ bool mysql_rename_user(THD *thd, List <L
> >  
> >    VOID(pthread_mutex_unlock(&acl_cache->lock));
> >  
> > -  if (mysql_bin_log.is_open())
> > +  if (result)
> > +    my_error(ER_CANNOT_USER, MYF(0), "RENAME USER", wrong_users.c_ptr_safe());
> > +  
> > +  if (some_users_renamed && mysql_bin_log.is_open())
> >    {
> >      Query_log_event qinfo(thd, thd->query, thd->query_length, 0, FALSE);
> >      mysql_bin_log.write(&qinfo);
> > @@ -5522,8 +5534,6 @@ bool mysql_rename_user(THD *thd, List <L
> >  
> >    rw_unlock(&LOCK_grant);
> >    close_thread_tables(thd);
> > -  if (result)
> > -    my_error(ER_CANNOT_USER, MYF(0), "RENAME USER", wrong_users.c_ptr_safe());
> >    DBUG_RETURN(result);
> >  }
> >  
> >
> >   
> 
> 

Thread
bk commit into 5.0 tree (hezx:1.2565) BUG#33862hezx17 Jan
  • Re: bk commit into 5.0 tree (hezx:1.2565) BUG#33862Mats Kindahl18 Jan
    • Re: bk commit into 5.0 tree (hezx:1.2565) BUG#33862He Zhenxing20 Jan
  • Re: bk commit into 5.0 tree (hezx:1.2565) BUG#33862Andrei Elkin24 Jan