List:Commits« Previous MessageNext Message »
From:Mats Kindahl Date:December 10 2009 12:05pm
Subject:Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151
View as plain text  

Alfranio Correia wrote:
> Hi Mats,
> 
> Thank you for the prompt reply.
> See some comments in-line
> 
[snip]

>>>> +  uint row_pack_length() { return pack_length(); }
>>>> +  uint32 pack_length_from_metadata(uint field_metadata) {
>>>> +    uint32 length= pack_length();
>>>> +    DBUG_PRINT("result", ("pack_length_from_metadata(%d): %u",
>>>> +                          field_metadata, length));
>>>> +    return length;
>>>> +  }
>>> Why do you need a different method? Is this just for debugging?
>> Field::pack_length_from_metadata() returns field_metadata by default, which does
>> not work. It need to return the pack length of the field.
> 
> 
> Both row_pack_length and pack_length_from_metadata() rely on the pack_length to
> return the value. The difference is the parameter in pack_length_from_metadata()
> which is only used for debugging.

Ah, you mean why there are both row_pack_length() and pack_length_from_metadata()?

The pack_length_from_metadata() is used when decoding the field from information
in the table map event. The row_pack_length() is needed for some of the fields
to do an accurate comparison.

These functions precede the worklog and is part of the code base from before
this patch. I just added them here since they were missing.

Ideally, the code should be slightly refactored to not have so many functions
for this job, but that is beyond the scope of this worklog.
[snip]

>>>> +      case MYSQL_TYPE_BIT:
>>>> +      {
>>>> +        uint16 x= field_metadata[index++]; 
>>>> +        x = x + (field_metadata[index++] << 8U);
>>> Is this a mistake? Note that the most significative byte is in the second
> part of the array.
>>> In the other cases, it is always in the first part.
>> Eh, not sure I follow you.
> 
> When there is two bytes to represent metadata, the most significant bits come first.
> 
> uint16 x= field_metadata[index++] << 8U.
> x = x + field_metadata[index++];

No. In the binary log, the metadata is stored little-endian, but when read up
into the server, the metdata is saved in "correct" order in the array inside
table_def.

[snip]

> I just tired. Forget about it.

OK. :)

> 
>> Please elaborate.
>>
>> This code was just copied from the header file.
>>
>> [snip]
>>
>>>> +
>>>> +table_def::~table_def()
>>>> +{
>>>> +  my_free(m_memory, MYF(0));
>>>> +#ifndef DBUG_OFF
>>>> +  m_type= 0;
>>>> +  m_size= 0;
>>>> +#endif
>>> What are you trying to prevent with this?
>> Discovering memory released twice in debug builds.
> 
> 
> Sorry, I still did not get how this can be useful to check if the memory is released
> twice.

No, you're right. There's no point in having these assignments.

/Matz

-- 
Mats Kindahl
Senior Software Engineer
Database Technology Group
Sun Microsystems
Thread
bzr commit into mysql-5.1 branch (mats:3180) WL#5151Mats Kindahl4 Dec
  • Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151Alfranio Correia10 Dec
    • Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151Sergei Golubchik10 Dec
    • Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151Mats Kindahl10 Dec
      • Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151Alfranio Correia10 Dec
        • Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151Mats Kindahl10 Dec
  • Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151Luís Soares10 Dec
    • Re: bzr commit into mysql-5.1 branch (mats:3180) WL#5151Mats Kindahl10 Dec