Re: bzr commit into mysql-6.0-backup branch (Rafal.Somla:2741) Bug#32702 WL#4644

[Rafal Somla]

Hi Chuck,

I have updated my patch to the latest 6.0-backup tree 
(revid:jorgen.loland@stripped).

I have addressed the issues you raised. As discussed in Stockholm I created a 
branch with my patch - you can pull changes from there and then all renames 
should be correctly handled by bzr. The url of the branch is:

bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-6.0-backup-bug32702

I'm waiting for your review of the updated patch.

Chuck Bell wrote:
> REQUESTS
> --------
> 1. The patch does not apply cleanly to bml.h or backup_logs.result.

You can pull it now from the branch I created.

> 2. The backup_errors test fails.

Now the result file should be correct. On my box all backup tests pass fine.

> 3. Inconsistent use of rename 'ddl_blocker' to 'bml_blocker'.

Fixed.

> 4. Unclear benefits of new 'prepared' state.

I removed the new state.

> 5. Move BML utility functions to si_objects.

I disagree - see my reply below.

> DETAILS
> -------
...
>> === modified file 'sql/sql_parse.cc'
>> --- a/sql/sql_parse.cc    2008-12-04 23:14:30 +0000
>> +++ b/sql/sql_parse.cc    2008-12-17 14:32:46 +0000
> 
> ...
> 
>> @@ -351,6 +392,13 @@ bool is_log_table_write_query(enum enum_
>>    return (sql_command_flags[command] & CF_WRITE_LOGS_COMMAND) != 0;
>>  }
>>  
>> +static
>> +bool is_bml_blocked(const enum enum_sql_command command)
>> +{
>> +  DBUG_ASSERT(command <= SQLCOM_END);
>> +  return (sql_command_flags[command] & CF_BLOCKED_BY_BML) != 0;
>> +}
>> +
> 
> [5] I think this belongs in si_objects. It could be useful in other 
> parts of backup code.
>

This function is analogous to two other functions introduced here: 
is_update_query() and is_log_table_write_query() therefore I think it should 
stay together with them.

Whether it belongs to si_objects depends on how you look at it. I see it as 
auxiliary function for reading the sql_command_flags[] table (hiding details 
such as the exact flag used to mark these commands). It is not part of the 
public interface of the BML service (as per design in the WL). It is rather part 
of the internal implementation of this service.

This implementation is spread across si_objects.cc where public interface 
methods bml_get(), bml_release() etc. are implemented), bml.cc where the 
BML_class is implemented and sql_parse.cc where the parser is modified to 
perform registering/de-registering of the DDL statements with the BML_class 
instance. Recognizing which statements are to be registered belongs, in my 
opinion, to the sql_parse.cc part of the implementation.

Rafal