Re: bzr commit into mysql-5.1 branch (mats:2683) Bug#38360

[Mats Kindahl]

Andrei Elkin wrote:
> Mats, hello.
> 
> The patch looks good, I have only minor comments about cset comments
> and tests. We might have been lucky if changes in log_event.cc
> actually fix another bug.

[snip]

>>   sql/log_event.cc
>>     Correcting code to only touch filler bits and leave
>>     all other bits alone. It is necessary since there is
>>     no guarantee that the engine will be able to fill in
>>     the bits correctly (e.g., the blackhole engine).
> 
> Chages in that part does not seem to relate to the current bug, but
> maybe they will fix Bug #39753.

Don't think so, it is related to range search. This is a patch in the part that
handles index search. However, I cannot fix this bug without adding this patch.

I'll see if I can commit this cset as a separate patch for that bug, it might be
BUG#39752 (!).

[snip]

>> === modified file 'mysql-test/suite/binlog/t/binlog_multi_engine.test'
>> --- a/mysql-test/suite/binlog/t/binlog_multi_engine.test	2008-02-28 21:50:15
> +0000
>> +++ b/mysql-test/suite/binlog/t/binlog_multi_engine.test	2008-09-22 19:00:29
> +0000
>> @@ -69,8 +69,11 @@ UPDATE t1m, t1n SET m = 2, e = 3 WHERE n
>>  
>>  #UPDATE t1m, t1n SET m = 2, e = 3 WHERE n = f;
>>  
>> -error ER_BINLOG_LOGGING_IMPOSSIBLE;
>> -UPDATE t1n, t1b SET e = 2, b = 3 WHERE f = c;
>> +# Blackhole now supports row-based logging, so we have no engine that
>> +# doesn't support it.
>> +
>> +#error ER_BINLOG_LOGGING_IMPOSSIBLE;
>> +#UPDATE t1n, t1b SET e = 2, b = 3 WHERE f = c;
> 
> I think it's better to remove this snippet instead of commenting-out,
> and make sure we have in the test one row-based-loggable sample for each
> UPDATE, DELETE, INSERT on a blackhole case.
> 
> There is no DELETE case as far as I can see.

True. I have a separate test for this, which I failed to add before committing.

>> === modified file 'sql/log_event.cc'
>> --- a/sql/log_event.cc	2008-09-03 10:01:18 +0000
>> +++ b/sql/log_event.cc	2008-09-22 19:00:29 +0000
>> @@ -8605,10 +8605,10 @@ int Rows_log_event::find_row(const Relay
>>        the necessary bits on the bytes and don't set the filler bits
>>        correctly.
>>      */
>> -    my_ptrdiff_t const pos=
>> -      table->s->null_bytes > 0 ? table->s->null_bytes - 1 : 0;
>> -    table->record[0][pos]= 0xFF;
>> -    
>> +    if (table->s->null_bytes > 0)
>> +      table->record[0][table->s->null_bytes - 1]|=
>> +        256U - (1U << table->s->last_null_bit_pos);
>> +
> 
> As noted earlier, I'd be awesome if you have fixed another bug.
> 

We'll see. I'll investigate further.

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