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?
