Hi Mats,
On 10/27/2010 01:55 PM, Mats Kindahl wrote:
> On 10/20/2010 04:45 PM, Luis Soares wrote:
>> #At
> file:///home/lsoares/Workspace/bzr/work/bugfixing/53386/mysql-trunk-bugfixing/ based on
> revid:alfranio.correia@stripped
>>
>> 3288 Luis Soares 2010-10-20
>> BUG#53386: HA_ERR_CORRUPT_EVENT or debug assertion with different
>> table on slave
>>
>> When the number of raw bytes used for a CHAR field exceeds 255
>> two bytes are actually used while packing the length of the
>> field. CHAR fields can exceed the raw 255 byte length, for
>> instance, if using multi-byte character sets.
>>
>> The problem is that when calculating the raw length of the field,
>> at unpacking time, only one byte was assumed to be used
>> always. Therefore, the number of bytes used would not add up,
>> triggering the assertion.
>>
>> We fix this, by making the member function that calculates the
>> raw byte length for a given field to take into account the actual
>> number of bytes used while packing the field length.
>> @ mysql-test/extra/rpl_tests/rpl_extraMaster_Col.test
>> Extended the test case with one based on the bug report entry.
>>
[ snip ]
>>
>> === modified file 'sql/rpl_utility.cc'
>> --- a/sql/rpl_utility.cc 2010-07-16 21:00:50 +0000
>> +++ b/sql/rpl_utility.cc 2010-10-20 14:45:31 +0000
>> @@ -225,9 +225,10 @@ uint32 table_def::calc_field_size(uint c
>> /*
>> We are reading the actual size from the master_data record
>> because this field has the actual lengh stored in the first
>> - byte.
>> + one or two bytes.
>> */
>> - length= (uint) *master_data + 1;
>> + uint32 max_length= max_display_length_for_field(MYSQL_TYPE_STRING,
> m_field_metadata[col]);
>> + length= (uint) (max_length> 255 ? 2 : 1) + *master_data;
>>
>
> If two bytes are used to represent the length, I think you need to read
> two bytes from the master data to compute the length... but you're just
> reading one.
Duh! You are right!
> Shouldn't it be:
>
> if (max_length> 255)
> length= *master_data + *(master_data + 1)<< 8 + 2;
> else
> length = *master_data + 1;
>
> or
>
> length = *master_data + (max_length> 255 ? 2 + *(master_data + 1)
> << 8 : 1)
>
> whichever you prefer.
I'll go with:
+ length= max_display_length_for_field(MYSQL_TYPE_STRING, m_field_metadata[col]) >
255 ? 2 : 1;
+
+ /* As in Field_string::unpack */
+ length+= ((length == 1) ? *master_data : uint2korr(master_data));
Additionally, I have added one assertion while unpacking a row so
that, in debug builds, we always assert that unpack and
calc_field_size return the same amount of bytes. (and that revealed
another concealed issue - bad assertion when calculating the size
for varchar strings).
Please, find a new patch at: http://lists.mysql.com/commits/122298
Thanks for your valuable comments.
> Just my few cents,
> Mats Kindahl
>
>
>> DBUG_ASSERT(length != 0);
>> }
>> break;
>>
>>
>>
>>
>>
>>
>
