List:Commits« Previous MessageNext Message »
From:Daogang Qu Date:January 14 2010 10:12am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3302)
Bug#49132
View as plain text  
Hi Libing,
Thanks for these good comments.
Please review a new patch base on them.

Best Regards,

Daogang

Libing Song wrote:
> 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.
>>     
>>>   
>>>       
>>     
>
>
>   

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