List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:October 13 2010 2:08pm
Subject:Re: bzr commit into mysql-5.5 branch (mats.kindahl:3089) Bug#52131
View as plain text  
Hi Mats,

Excellent work.
Patch conditionally approved.

I am missing a test case.
See also a minor comment in-line

Cheers

On 10/06/2010 06:20 PM, Mats Kindahl wrote:
> #At file:///home/bzr/bugs/b52131-5.5/ based on
> revid:alexander.nozdrin@stripped
>
>   3089 Mats Kindahl	2010-10-06
>        Bug #52131: SET and ENUM stored endian-dependent in binary log
>
>        Replication SET and ENUM fields from a big-endian to a little-
>        endian machine (or the opposite) that are represented using
>        more than 1 byte (SET fields with more than 8 members or ENUM
>        fields with more than 256 constants) will fail to replicate
>        correctly when using row-based replication.
>
>        The reason is that there are no pack() or unpack() functions
>        for Field_set or Field_enum, which make them rely on Field::pack
>        and Field::unpack. These functions pack data as strings, but
>        since Field_set and Field_enum use integral types for
>        representation, the fields are stored incorrectly on big-endian
>        machines.
>
>        This patch adds Field_enum::pack and Field_enum::unpack
>        functions that store the integral value correctly in the binary
>        log even on big-endian machines. Since Field_set inherits from
>        Field_enum, it will use the same functions for packing and
>        unpacking the field.
>       @ sql/field.cc
>          Removing some obsolete debug printouts and adding Field_enum::pack
>          and Field_enum::unpack functions.
>       @ sql/field.h
>          Adding helper functions for packing and unpacking 16- and
>          24-bit integral types.
>
>          Field_short::pack and Field_short::unpack now use these functions.
>       @ sql/rpl_record.cc
>          Removing some obsolete debug printouts and adding some
>          more useful ones.
>
>      modified:
>        sql/field.cc
>        sql/field.h
>        sql/rpl_record.cc
> === modified file 'sql/field.cc'
> --- a/sql/field.cc	2010-09-28 15:15:58 +0000
> +++ b/sql/field.cc	2010-10-06 17:20:18 +0000
> @@ -7725,12 +7725,6 @@ void Field_blob::sql_type(String&res) c
>   uchar *Field_blob::pack(uchar *to, const uchar *from,
>                           uint max_length, bool low_byte_first)
>   {
> -  DBUG_ENTER("Field_blob::pack");
> -  DBUG_PRINT("enter", ("to: 0x%lx; from: 0x%lx;"
> -                       " max_length: %u; low_byte_first: %d",
> -                       (ulong) to, (ulong) from,
> -                       max_length, low_byte_first));
> -  DBUG_DUMP("record", from, table->s->reclength);
>     uchar *save= ptr;
>     ptr= (uchar*) from;
>     uint32 length=get_length();			// Length of from string
> @@ -7751,8 +7745,7 @@ uchar *Field_blob::pack(uchar *to, const
>       memcpy(to+packlength, from,length);
>     }
>     ptr=save;					// Restore org row pointer
> -  DBUG_DUMP("packed", to, packlength + length);
> -  DBUG_RETURN(to+packlength+length);
> +  return to+packlength+length;
>   }

Why don't you keep the DBUG_ENTER and DBUG_RETURN?
Any reason to remove them?
Thread
bzr commit into mysql-5.5 branch (mats.kindahl:3089) Bug#52131Mats Kindahl6 Oct
Re: bzr commit into mysql-5.5 branch (mats.kindahl:3089) Bug#52131Alfranio Correia13 Oct
  • Re: bzr commit into mysql-5.5 branch (mats.kindahl:3089) Bug#52131Mats Kindahl21 Oct
Re: bzr commit into mysql-5.5 branch (mats.kindahl:3089) Bug#52131Mats Kindahl26 Oct