Re: bzr commit into mysql-5.1 branch (mats:2771) Bug#39934

[Mats Kindahl]

Sven Sandberg wrote:
> Mats Kindahl wrote:

[snip]

>> === modified file 'sql/sql_base.cc'
>> --- a/sql/sql_base.cc	2008-10-08 08:46:25 +0000
>> +++ b/sql/sql_base.cc	2008-11-17 10:34:29 +0000
>> @@ -30,6 +30,13 @@
>>  
>>  #define FLAGSTR(S,F) ((S) & (F) ? #F " " : "")
>>  
>> +static const char *prelocked_mode_name[] = {
>> +  "NON_PRELOCKED",
>> +  "PRELOCKED",
>> +  "PRELOCKED_UNDER_LOCK_TABLES",
>> +};
> 
> This is only used for one DBUG_PRINT? I suggest moving the array
> declaration to where it is used:
> 
>   #ifdef DEBUG
>   {
>     static const char *prelocked_mode_name = { ... };
>     DBUG_PRINT(...);
>   }
>   #endif

I made it local to the function.

[snip]

>> @@ -5107,27 +5140,63 @@ int decide_logging_format(THD *thd, TABL
>>      DBUG_PRINT("info", ("multi_engine: %s",
>>                          multi_engine ? "TRUE" : "FALSE"));
>>  
>> +    thd->set_flags_all_set(flags_all_set);
>> +
> 
> (*) It is a little bit ugly to save flags_all_set here. It's just an
> internal state used in a computation. Why not determine the warning in
> this function, and save a pointer to the warning text in the THD object
> instead of the intersection of all flags? That would ensure all logic is
> in this method, and binlog_query only does the reporting (so less
> interface changes in case the logic needs to change later).

I tried this approach, but unfortunately the management of memory and printing
the correct error message becomes contorted for SQL containing nested function
and procedure calls.

Best wishes,
Mats Kindahl
-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com