List:Commits« Previous MessageNext Message »
From:Libing Song Date:January 14 2010 5:54am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3302)
Bug#49132
View as plain text  
On Thu, 2010-01-14 at 11:09 +0800, Daogang Qu wrote:
> Libing Song wrote:
> > Hi Daogang,
> >
> >   Nice Work. Please find my review comments below.
> >
> > STATUS
> > ------
> >   Not Approved.
> >
> > REQUIRED CHANGES
> > ----------------
> >   RC1. Please use upper case for all statements.
> >   
> What's upper case?
I mean 'CREATE TABLE' is better than 'create table'.

> >   RC2. 
> >     GRANT ... ON {TABLE | FUNCTION | PROCEDURE} ...
> >     REVOKE ... FROM {TABLE | FUNCTION | PROCEDURE} ...
> >     Above statements call different function to handle it.
> >     So it is better to test on all occasions.
> >   
> GRANT and REVOKE are DDL statement too. We tested GRANT and REVOKE here.
> So it's not necessary to test on all occasions. And the table, function
> and procedure
> are tested.
I mean we shall test 'GRANT ...ON TABLE t1 ...' and 'GRANT ... ON
FUNCTION f1 ...', the first statement calls mysql_grant_table function
and the second statement calls mysql_grant_func function and other
statements call mysql_grant function.

> >   RC3. I think we did not need
> > extra/rpl_tests/rpl_tmp_table_and_DDL_ACL.test and
> > extra/rpl_tests/rpl_tmp_table_and_DDL.test.
> >
> > It is simpler in one file like the following:
> >   CREATE TEMPORARY TABLE t1 (c1 INT);
> >   
> >   CREATE USER 'test_1'@'fakehost';
> >   INSERT INTO t1 VALUES(1);
> >
> >   GRANT ALL PRIVILEGES ON *.* TO 'test_1'@'fakehost';
> >   INSERT INTO t1 VALUES(1);
> >  
> >   ......
> >
> >
> >   DROP TEMPORARY TABLE t1;  
> >   
> Good idea!
> >    
> >   RC4. There is a indent error.
> >   
> Thanks for the reminder!
> > REQUESTS
> > --------
> >  n/a
> >
> > SUGGESTIONS
> > -----------
> >  n/a
> >
> > DETAILS 
> >
> > On Tue, 2010-01-05 at 10:00 +0000, Dao-Gang.Qu@stripped wrote: 
> >   
> >> #At file:///home/daogangqu/mysql/bzrwork/bug49132/mysql-5.1-bugteam/ based
> on revid:ramil@stripped
> >>
> >>  3302 Dao-Gang.Qu@stripped	2010-01-05
> >>       Bug #49132  	Replication failure on temporary table + DDL
> >>       
> >>       In RBR, DDL statement will change binlog format to non row-based
> >>       format, and then manipulating a temporary table can not reset binlog
> >>       format to row-based format rightly. So that the manipulated statement
> >>       is binlogged with statement-based format.
> >>    
> >>     
> > It is better to explain why DDL statements change binlog format and
> > forget to set it back.
> >
> >   
> It's better.
> >   
> >>  
> >> @@ -674,6 +686,8 @@ Events::drop_event(THD *thd, LEX_STRING 
> >>      write_bin_log(thd, TRUE, thd->query(), thd->query_length());
> >>    }
> >>    pthread_mutex_unlock(&LOCK_event_metadata);
> >> +  /* Restore the state of binlog format */
> >> +    thd->current_stmt_binlog_row_based= save_binlog_row_based;
> >>     
> > Please adjust the indent of above line. 
> >   
> Sure.
> >
> >
> >   
> 
> 


-- 
Your Sincerely,
Libing Song
==================================
MySQL Replication Team
Software Engineer
Certified (ISC)2 CISSP

Email : Li-Bing.Song@stripped
Skype : libing.song
MSN   : slb_database@stripped
Phone : +86 010-6505-4020 ext. 319
Mobile: +86 138-1144-2038
==================================


Thread
bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3302) Bug#49132Dao-Gang.Qu5 Jan
  • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3302)Bug#49132Libing Song8 Jan
    • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3302)Bug#49132Daogang Qu14 Jan
      • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3302)Bug#49132Libing Song14 Jan
        • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3302)Bug#49132Daogang Qu14 Jan