Hi Andrei!
Summary of comments below:
* I'll add Field_blob to the cset comment.
* I'll reuse Field::pack() and Field::unpack() where appropriate
* I will change the code in pack_row() to take the maximum field
size into consideration instead of assuming a maximum.
* I'll add an assertion to the locations where the WORDS_BIGENDIAN
removes code.
Could you verify that this suffices?
Just my few cents,
Mats Kindahl
Andrei Elkin wrote:
> Mats, hej.
>
> Hopefully i have my final set of questions which do not undermine
> the value of your work.
>
>
>
>> Below is the list of changes that have just been committed into a local
>> 5.1 repository of mats. When mats does a push these changes will
>> be propagated to the main repository and, within 24 hours after the
>> push, to the public repository.
>> For information on how to access the public repository
>> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>>
>> ChangeSet@stripped, 2007-10-05 18:16:11+02:00, mats@stripped +20
> -0
>> BUG#29549 (Endians: test failures on Solaris):
>>
>> Refactoring code to add parameter to pack() and unpack() functions with
>> purpose of indicating if data should be packed in little-endian or
>> native order. Using new functions to always pack data for binary log
>> in little-endian order. The purpose of this refactoring is to allow
>> proper implementation of endian-agnostic pack() and unpack() functions.
>>
>> Eliminating several versions of virtual pack() and unpack() functions
>> in favor for one single virtual function which is overridden in
>> subclasses.
>>
>> Implementing pack() and unpack() functions for some field types that
>> packed data in native format regardless of the value of the
>> st_table_share::db_low_byte_first flag.
>>
>> The field types that were packed in native format regardless are:
>> Field_real, Field_decimal, Field_tiny, Field_short, Field_medium,
>> Field_long, and Field_longlong.
>>
>>
>
> Could you please either add Field_blob or refuse with an explanation
> to me. There is a hunk which makes me thinking Field_blob packing has been
> corrected too.
>
I'll add it.
[snip]
>
>> Using pad_char for the field instead of spaces (0x20) when
>> unpacking.
>>
>
> You answered that the change is "obvious" and thereafter you were not
> going to expand the comments.
> After such answer I indeed stopped to find any other subttle reason. Thanks.
>
>
OK.
[snip]
>
>> -const uchar *Field_new_decimal::unpack(uchar* to,
>> - const uchar *from,
>> - uint param_data)
>> -{
>> +const uchar *
>> +Field_new_decimal::unpack(uchar* to,
>> + const uchar *from,
>> + uint param_data,
>> + bool low_byte_first __attribute__((unused)))
>> +{
>> + if (param_data == 0)
>> + {
>> + uint const length= pack_length();
>> + memcpy(to, from, pack_length());
>> + return from + length;
>> + }
>> +
>>
>
> The early return block corresponds to a branch of Field::unpack().
> To call the parent method is better instead of
> two maintenan two equivalent but separate pieces of code.
>
OK.
>
>
>> uint from_precision= (param_data & 0xff00) >> 8U;
>> uint from_decimal= param_data & 0x00ff;
>> uint length=pack_length();
>> @@ -3857,6 +3926,49 @@ void Field_longlong::sql_type(String &re
>> }
>>
>>
>> +/*
>> + Floating-point numbers
>> + */
>> +
>> +uchar *
>> +Field_real::pack(uchar *to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> +{
>> + DBUG_ENTER("Field_real::pack");
>> + DBUG_ASSERT(max_length >= pack_length());
>> + DBUG_PRINT("debug", ("pack_length(): %u", pack_length()));
>> +#ifdef WORDS_BIGENDIAN
>> + if (low_byte_first != table->s->db_low_byte_first)
>> + {
>> + const uchar *dptr= from + pack_length();
>> + while (dptr-- > from)
>> + *to++ = *dptr;
>> + DBUG_RETURN(to);
>> + }
>> + else
>> +#endif
>>
>
>
> Obviously, there will be change in behaviour for packing in the native format on
> the big endian.
> There is a change for swapping whereas previously the two arg
> Field_real::pack method never did not do that.
>
> Might it to be an issue?
>
It was an issue in the previous code: replication for NDB failed on big
endian machines, that is why this code was added.
[snip]
>
>> +
>> + /*
>> + Store max length, which will occupy packlength bytes. If the max
>> + length given is smaller than the actual length of the blob, we
>> + just store the initial bytes of the blob.
>>
>
> Naturallly, a question pops up:
> and what will happend with the rest
> lenth - max_length bytes of the field? Will it be ever packed
> (replicated)?
>
No, they will be truncated. Actually, it seems to be a problem in the
old code in that the max length was given by a constant (UINT_MAX), and
not dependent on the max field size for the given field.
I'm adding a fix for that one as well, but that requires a little more work.
>
>
>> + */
>> + store_length(to, packlength, min(length, max_length), low_byte_first);
>> +
>> + /*
>> + Store the actual blob data, which will occupy 'length' bytes.
>> + */
>> + if (length > 0)
>> {
>> get_ptr((uchar**) &from);
>> memcpy(to+packlength, from,length);
>> }
>> ptr=save; // Restore org row pointer
>> - return to+packlength+length;
>> + DBUG_DUMP("packed", to, packlength + length);
>> + DBUG_RETURN(to+packlength+length);
>> }
>>
>>
>> @@ -7644,28 +7756,26 @@ uchar *Field_blob::pack(uchar *to, const
>>
>> @param to Destination of the data
>> @param from Source of the data
>> - @param param_data <not used>
>>
>> @return New pointer into memory based on from + length of the data
>> */
>> const uchar *Field_blob::unpack(uchar *to,
>> const uchar *from,
>> - uint param_data)
>> + uint param_data,
>> + bool low_byte_first)
>> {
>> - return unpack(to, from);
>> -}
>> -
>> -
>> -const uchar *Field_blob::unpack(uchar *to, const uchar *from)
>> -{
>> - memcpy(to,from,packlength);
>> - uint32 length=get_length(from);
>> - from+=packlength;
>> - if (length)
>> - memcpy_fixed(to+packlength, &from, sizeof(from));
>> - else
>> - bzero(to+packlength,sizeof(from));
>> - return from+length;
>> + DBUG_ENTER("Field_blob::unpack");
>> + DBUG_PRINT("enter", ("to: 0x%lx; from: 0x%lx;"
>> + " param_data: %u; low_byte_first: %d",
>> + to, from, param_data, low_byte_first));
>> + uint const master_packlength=
>> + param_data > 0 ? param_data & 0xFF : packlength;
>> + uint32 const length= get_length(from, master_packlength, low_byte_first);
>> + DBUG_DUMP("packed", from, length + master_packlength);
>> + store(reinterpret_cast<const char*>(from) + master_packlength,
>> + length, field_charset);
>> + DBUG_DUMP("record", to, table->s->reclength);
>> + DBUG_RETURN(from + master_packlength + length);
>> }
>>
>> /* Keys for blobs are like keys on varchars */
>>
>
> You need to update the comments in 4-args version of
> Field_blob::unpack (perhaps in some other as well, please check it
> too) continuing to say the method is replication-related.
>
The methods are not pure replication. They are used by filesort as well.
>
>
>> @@ -7715,7 +7825,9 @@ int Field_blob::pack_cmp(const uchar *b,
>>
>> /* Create a packed key that will be used for storage from a MySQL row */
>>
>> -uchar *Field_blob::pack_key(uchar *to, const uchar *from, uint max_length)
>> +uchar *
>> +Field_blob::pack_key(uchar *to, const uchar *from, uint max_length,
>> + bool low_byte_first __attribute__((unused)))
>> {
>> uchar *save= ptr;
>> ptr= (uchar*) from;
>> @@ -7760,8 +7872,9 @@ uchar *Field_blob::pack_key(uchar *to, c
>> Pointer into 'from' past the last byte copied from packed key.
>> */
>>
>> -const uchar *Field_blob::unpack_key(uchar *to, const uchar *from,
>> - uint max_length)
>> +const uchar *
>> +Field_blob::unpack_key(uchar *to, const uchar *from, uint max_length,
>> + bool low_byte_first __attribute__((unused)))
>> {
>> /* get length of the blob key */
>> uint32 length= *from++;
>> @@ -7784,8 +7897,9 @@ const uchar *Field_blob::unpack_key(ucha
>>
>> /* Create a packed key that will be used for storage from a MySQL key */
>>
>> -uchar *Field_blob::pack_key_from_key_image(uchar *to, const uchar *from,
>> - uint max_length)
>> +uchar *
>> +Field_blob::pack_key_from_key_image(uchar *to, const uchar *from, uint
> max_length,
>> + bool low_byte_first
> __attribute__((unused)))
>> {
>> uint length=uint2korr(from);
>> if (length > max_length)
>> @@ -8672,9 +8786,11 @@ void Field_bit::sql_type(String &res) co
>> }
>>
>>
>> -uchar *Field_bit::pack(uchar *to, const uchar *from, uint max_length)
>> +uchar *
>> +Field_bit::pack(uchar *to, const uchar *from, uint max_length,
>> + bool low_byte_first __attribute__((unused)))
>> {
>> - DBUG_ASSERT(max_length);
>> + DBUG_ASSERT(max_length > 0);
>> uint length;
>> if (bit_len > 0)
>> {
>> @@ -8709,28 +8825,44 @@ uchar *Field_bit::pack(uchar *to, const
>> /**
>> Unpack a bit field from row data.
>>
>> - This method is used to unpack a bit field from a master whose size
>> + This method is used to unpack a bit field from a master whose size
>> of the field is less than that of the slave.
>> -
>> +
>> @param to Destination of the data
>> @param from Source of the data
>> @param param_data Bit length (upper) and length (lower) values
>>
>> @return New pointer into memory based on from + length of the data
>> */
>> -const uchar *Field_bit::unpack(uchar *to,
>> - const uchar *from,
>> - uint param_data)
>> +const uchar *
>> +Field_bit::unpack(uchar *to, const uchar *from, uint param_data,
>> + bool low_byte_first __attribute__((unused)))
>> {
>> uint const from_len= (param_data >> 8U) & 0x00ff;
>> uint const from_bit_len= param_data & 0x00ff;
>> /*
>> - If the master and slave have the same sizes, then use the old
>> - unpack() method.
>> + If the parameter data is zero (i.e., undefined), or if the master
>> + and slave have the same sizes, then use the old unpack() method.
>> */
>> - if ((from_bit_len == bit_len) &&
>> - (from_len == bytes_in_rec))
>> - return(unpack(to, from));
>> + if (param_data == 0 ||
>> + (from_bit_len == bit_len) && (from_len == bytes_in_rec))
>> + {
>> + if (bit_len > 0)
>> + {
>> + /*
>> + set_rec_bits is a macro, don't put the post-increment in the
>> + argument since that might cause strange side-effects.
>> +
>> + For the choice of the second argument, see the explanation for
>> + Field_bit::pack().
>> + */
>> + set_rec_bits(*from, bit_ptr + (to - ptr), bit_ofs, bit_len);
>> + from++;
>> + }
>> + memcpy(to, from, bytes_in_rec);
>> + return from + bytes_in_rec;
>> + }
>> +
>> /*
>> We are converting a smaller bit field to a larger one here.
>> To do that, we first need to construct a raw value for the original
>> @@ -8755,25 +8887,6 @@ const uchar *Field_bit::unpack(uchar *to
>> store(value, new_len, system_charset_info);
>> my_afree(value);
>> return from + len;
>> -}
>> -
>> -
>> -const uchar *Field_bit::unpack(uchar *to, const uchar *from)
>> -{
>> - if (bit_len > 0)
>> - {
>> - /*
>> - set_rec_bits is a macro, don't put the post-increment in the
>> - argument since that might cause strange side-effects.
>> -
>> - For the choice of the second argument, see the explanation for
>> - Field_bit::pack().
>> - */
>> - set_rec_bits(*from, bit_ptr + (to - ptr), bit_ofs, bit_len);
>> - from++;
>> - }
>> - memcpy(to, from, bytes_in_rec);
>> - return from + bytes_in_rec;
>> }
>>
>>
>> diff -Nrup a/sql/field.h b/sql/field.h
>> --- a/sql/field.h 2007-08-04 11:08:07 +02:00
>> +++ b/sql/field.h 2007-10-05 18:16:00 +02:00
>> @@ -339,32 +339,45 @@ public:
>> return str;
>> }
>> virtual bool send_binary(Protocol *protocol);
>> - virtual uchar *pack(uchar *to, const uchar *from, uint max_length=~(uint) 0)
>> +
>> + virtual uchar *pack(uchar *to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> + /**
>> + @overload Field::pack(uchar*, const uchar*, uint, bool)
>> + */
>> + uchar *pack(uchar *to, const uchar *from)
>> {
>> - uint32 length=pack_length();
>> - memcpy(to,from,length);
>> - return to+length;
>> + DBUG_ENTER("Field::pack");
>> + uchar *result= this->pack(to, from, UINT_MAX,
> table->s->db_low_byte_first);
>> + DBUG_RETURN(result);
>> }
>> - virtual const uchar *unpack(uchar* to, const uchar *from, uint param_data);
>> - virtual const uchar *unpack(uchar* to, const uchar *from)
>> +
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first);
>> + /**
>> + @overload Field::unpack(uchar*, const uchar*, uint, bool)
>> + */
>> + const uchar *unpack(uchar* to, const uchar *from)
>> {
>> - uint length=pack_length();
>> - memcpy(to,from,length);
>> - return from+length;
>> + DBUG_ENTER("Field::unpack");
>> + const uchar *result= unpack(to, from, 0U,
> table->s->db_low_byte_first);
>> + DBUG_RETURN(result);
>> }
>>
>
>
>> - virtual uchar *pack_key(uchar* to, const uchar *from, uint max_length)
>> +
>> + virtual uchar *pack_key(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> {
>> - return pack(to,from,max_length);
>> + return pack(to, from, max_length, low_byte_first);
>> }
>>
>
> Previously, pack_key was defined to work in native format.
> Why not
>
>> + return pack(to, from, max_length, 0);
>>
> and to leave intact the signature?
> The question is asked also because all overloaded definitions does not
> use the last new arg.
>
> Similar applies to pack_key_from_key_image.
>
For symmetry reasons, I have elected to use the same signature for all
the functions. The fact that they are not used currently does not
preclude that they will be used in the future. Since this patch
simplifies the pack()/unpack() functionality and base it on a single
function, then by symmetry, I should follow the same pattern for all
similar functions. This makes it easier to use the interface. The fact
that it uses pack() internally, I view as an implementation issue.
>
>> virtual uchar *pack_key_from_key_image(uchar* to, const uchar *from,
>> - uint max_length)
>> + uint max_length, bool low_byte_first)
>> {
>> - return pack(to,from,max_length);
>> + return pack(to, from, max_length, low_byte_first);
>> }
>> virtual const uchar *unpack_key(uchar* to, const uchar *from,
>> - uint max_length)
>> + uint max_length, bool low_byte_first)
>> {
>> - return unpack(to,from);
>> + return unpack(to, from, max_length, low_byte_first);
>> }
>> virtual uint packed_col_length(const uchar *to, uint length)
>> { return length;}
>>
>
>
>
>> @@ -556,6 +569,10 @@ public:
>> int truncate(double *nr, double max_length);
>> uint32 max_display_length() { return field_length; }
>> uint size_of() const { return sizeof(*this); }
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first);
>> + virtual uchar *pack(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> };
>>
>>
>>
>
>
>> @@ -584,6 +601,16 @@ public:
>> void overflow(bool negative);
>> bool zero_pack() const { return 0; }
>> void sql_type(String &str) const;
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first)
>> + {
>> + return Field::unpack(to, from, param_data, low_byte_first);
>> + }
>> + virtual uchar *pack(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> + {
>> + return Field::pack(to, from, max_length, low_byte_first);
>> + }
>> };
>>
>>
>
> What's a point not to use automatic overloading?
>
Because there is a class in between that does not do the correct thing.
This class is not directly derived from Field.
>
>
>>
>> @@ -629,7 +656,8 @@ public:
>> uint size_of() const { return sizeof(*this); }
>> uint32 pack_length() const { return (uint32) bin_size; }
>> uint is_equal(Create_field *new_field);
>> - virtual const uchar *unpack(uchar* to, const uchar *from, uint param_data);
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first);
>> };
>>
>>
>>
>
> The same as before.
>
... and the same answer as before.
>
>> @@ -660,6 +688,20 @@ public:
>> uint32 pack_length() const { return 1; }
>> void sql_type(String &str) const;
>> uint32 max_display_length() { return 4; }
>> +
>> + virtual uchar *pack(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> + {
>> + *to= *from;
>> + return to + 1;
>> + }
>> +
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first)
>> + {
>> + *to= *from;
>> + return from + 1;
>> + }
>> };
>>
>>
>> @@ -695,8 +737,47 @@ public:
>> uint32 pack_length() const { return 2; }
>> void sql_type(String &str) const;
>> uint32 max_display_length() { return 6; }
>> -};
>>
>> + virtual uchar *pack(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> + {
>> + int16 val;
>> +#ifdef WORDS_BIGENDIAN
>> + if (table->s->db_low_byte_first)
>> + val = sint2korr(from);
>> + else
>> +#endif
>> + shortget(val, from);
>> +
>> +#ifdef WORDS_BIGENDIAN
>> + if (low_byte_first)
>> + int2store(to, val);
>> + else
>> +#endif
>> + shortstore(to, val);
>> + return to + sizeof(val);
>> + }
>> +
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first)
>> + {
>> + int16 val;
>> +#ifdef WORDS_BIGENDIAN
>> + if (low_byte_first)
>> + val = sint2korr(from);
>> + else
>> +#endif
>> + shortget(val, from);
>> +
>> +#ifdef WORDS_BIGENDIAN
>> + if (table->s->db_low_byte_first)
>> + int2store(to, val);
>> + else
>> +#endif
>> + shortstore(to, val);
>> + return from + sizeof(val);
>> + }
>> +};
>>
>> class Field_medium :public Field_num {
>> public:
>>
>
>
>
>> @@ -725,6 +806,18 @@ public:
>> uint32 pack_length() const { return 3; }
>> void sql_type(String &str) const;
>> uint32 max_display_length() { return 8; }
>> +
>> + virtual uchar *pack(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> + {
>> + return Field::pack(to, from, max_length, low_byte_first);
>> + }
>> +
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first)
>> + {
>> + return Field::unpack(to, from, param_data, low_byte_first);
>> + }
>> };
>>
>>
>
> the same as before.
>
... and the same answer as before.
>
>
>
>> @@ -760,6 +853,45 @@ public:
>> uint32 pack_length() const { return 4; }
>> void sql_type(String &str) const;
>> uint32 max_display_length() { return MY_INT32_NUM_DECIMAL_DIGITS; }
>> + virtual uchar *pack(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> + {
>> + int32 val;
>> +#ifdef WORDS_BIGENDIAN
>> + if (table->s->db_low_byte_first)
>> + val = sint4korr(from);
>> + else
>> +#endif
>> + longget(val, from);
>> +
>> +#ifdef WORDS_BIGENDIAN
>> + if (low_byte_first)
>> + int4store(to, val);
>> + else
>> +#endif
>> + longstore(to, val);
>> + return to + sizeof(val);
>> + }
>>
>
> Obviously, the modified since the last version of the patch definition
> assumes that on the little endian
>
> table->s->db_low_byte_first is true
>
> Not to disagree I think we are better to be safe to assert that
> explicitly at all places where you oprimized out
> `if (table->s->db_low_byte_first)' check.
>
I can add assertion for that, yes. It is actually a good idea.
>
>
>> +
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first)
>> + {
>> + int32 val;
>> +#ifdef WORDS_BIGENDIAN
>> + if (low_byte_first)
>> + val = sint4korr(from);
>> + else
>> +#endif
>> + longget(val, from);
>> +
>> +#ifdef WORDS_BIGENDIAN
>> + if (table->s->db_low_byte_first)
>> + int4store(to, val);
>> + else
>> +#endif
>> + longstore(to, val);
>> + return from + sizeof(val);
>> + }
>> };
>>
>>
>> @@ -802,6 +934,45 @@ public:
>> void sql_type(String &str) const;
>> bool can_be_compared_as_longlong() const { return TRUE; }
>> uint32 max_display_length() { return 20; }
>> + virtual uchar *pack(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first)
>> + {
>> + int64 val;
>> +#ifdef WORDS_BIGENDIAN
>> + if (table->s->db_low_byte_first)
>> + val = sint8korr(from);
>> + else
>> +#endif
>> + longlongget(val, from);
>> +
>> +#ifdef WORDS_BIGENDIAN
>> + if (low_byte_first)
>> + int8store(to, val);
>> + else
>> +#endif
>> + longlongstore(to, val);
>> + return to + sizeof(val);
>> + }
>> +
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first)
>> + {
>> + int64 val;
>> +#ifdef WORDS_BIGENDIAN
>> + if (low_byte_first)
>> + val = sint8korr(from);
>> + else
>> +#endif
>> + longlongget(val, from);
>> +
>> +#ifdef WORDS_BIGENDIAN
>> + if (table->s->db_low_byte_first)
>> + int8store(to, val);
>> + else
>> +#endif
>> + longlongstore(to, val);
>> + return from + sizeof(val);
>> + }
>> };
>> #endif
>>
>> @@ -1176,9 +1347,10 @@ public:
>> int cmp(const uchar *,const uchar *);
>> void sort_string(uchar *buff,uint length);
>> void sql_type(String &str) const;
>> - uchar *pack(uchar *to, const uchar *from, uint max_length=~(uint) 0);
>> - virtual const uchar *unpack(uchar* to, const uchar *from, uint param_data);
>> - const uchar *unpack(uchar* to, const uchar *from);
>> + virtual uchar *pack(uchar *to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first);
>> int pack_cmp(const uchar *a,const uchar *b,uint key_length,
>> my_bool insert_or_update);
>> int pack_cmp(const uchar *b,uint key_length,my_bool insert_or_update);
>> @@ -1250,13 +1422,15 @@ public:
>> uint get_key_image(uchar *buff,uint length, imagetype type);
>> void set_key_image(const uchar *buff,uint length);
>> void sql_type(String &str) const;
>> - uchar *pack(uchar *to, const uchar *from, uint max_length=~(uint) 0);
>> - uchar *pack_key(uchar *to, const uchar *from, uint max_length);
>> + virtual uchar *pack(uchar *to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> + uchar *pack_key(uchar *to, const uchar *from, uint max_length, bool
> low_byte_first);
>> uchar *pack_key_from_key_image(uchar* to, const uchar *from,
>> - uint max_length);
>> - virtual const uchar *unpack(uchar* to, const uchar *from, uint param_data);
>> - const uchar *unpack(uchar* to, const uchar *from);
>> - const uchar *unpack_key(uchar* to, const uchar *from, uint max_length);
>> + uint max_length, bool low_byte_first);
>> + virtual const uchar *unpack(uchar* to, const uchar *from,
>> + uint param_data, bool low_byte_first);
>> + const uchar *unpack_key(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> int pack_cmp(const uchar *a, const uchar *b, uint key_length,
>> my_bool insert_or_update);
>> int pack_cmp(const uchar *b, uint key_length,my_bool insert_or_update);
>> @@ -1374,13 +1548,13 @@ public:
>> @retval The length in the row plus the size of the data.
>> */
>> uint32 get_packed_size(const uchar *ptr_arg, bool low_byte_first)
>> - {return packlength + get_length(ptr_arg, low_byte_first);}
>> + {return packlength + get_length(ptr_arg, packlength, low_byte_first);}
>>
>> inline uint32 get_length(uint row_offset= 0)
>> - { return get_length(ptr+row_offset, table->s->db_low_byte_first); }
>> - uint32 get_length(const uchar *ptr, bool low_byte_first);
>> + { return get_length(ptr+row_offset, this->packlength,
> table->s->db_low_byte_first); }
>> + uint32 get_length(const uchar *ptr, uint packlength, bool low_byte_first);
>> uint32 get_length(const uchar *ptr_arg)
>> - { return get_length(ptr_arg, table->s->db_low_byte_first); }
>> + { return get_length(ptr_arg, this->packlength,
> table->s->db_low_byte_first); }
>> void put_length(uchar *pos, uint32 length);
>> inline void get_ptr(uchar **str)
>> {
>> @@ -1421,13 +1595,16 @@ public:
>> memcpy_fixed(ptr+packlength,&tmp,sizeof(char*));
>> return 0;
>> }
>> - uchar *pack(uchar *to, const uchar *from, uint max_length= ~(uint) 0);
>> - uchar *pack_key(uchar *to, const uchar *from, uint max_length);
>> + virtual uchar *pack(uchar *to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> + uchar *pack_key(uchar *to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> uchar *pack_key_from_key_image(uchar* to, const uchar *from,
>> - uint max_length);
>> - virtual const uchar *unpack(uchar *to, const uchar *from, uint param_data);
>> - const uchar *unpack(uchar *to, const uchar *from);
>> - const uchar *unpack_key(uchar* to, const uchar *from, uint max_length);
>> + uint max_length, bool low_byte_first);
>> + virtual const uchar *unpack(uchar *to, const uchar *from,
>> + uint param_data, bool low_byte_first);
>> + const uchar *unpack_key(uchar* to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> int pack_cmp(const uchar *a, const uchar *b, uint key_length,
>> my_bool insert_or_update);
>> int pack_cmp(const uchar *b, uint key_length,my_bool insert_or_update);
>> @@ -1602,9 +1779,10 @@ public:
>> uint32 pack_length() const { return (uint32) (field_length + 7) / 8; }
>> uint32 pack_length_in_rec() const { return bytes_in_rec; }
>> void sql_type(String &str) const;
>> - uchar *pack(uchar *to, const uchar *from, uint max_length=~(uint) 0);
>> - virtual const uchar *unpack(uchar *to, const uchar *from, uint param_data);
>> - const uchar *unpack(uchar* to, const uchar *from);
>> + virtual uchar *pack(uchar *to, const uchar *from,
>> + uint max_length, bool low_byte_first);
>> + virtual const uchar *unpack(uchar *to, const uchar *from,
>> + uint param_data, bool low_byte_first);
>> virtual void set_default();
>>
>> Field *new_key_field(MEM_ROOT *root, struct st_table *new_table,
>> diff -Nrup a/sql/log.cc b/sql/log.cc
>> --- a/sql/log.cc 2007-08-14 14:20:02 +02:00
>> +++ b/sql/log.cc 2007-10-05 18:16:01 +02:00
>> @@ -3571,9 +3571,6 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>> (!binlog_filter->db_ok(local_db)))
>> {
>> VOID(pthread_mutex_unlock(&LOCK_log));
>> - DBUG_PRINT("info",("OPTION_BIN_LOG is %s, db_ok('%s') == %d",
>> - (thd->options & OPTION_BIN_LOG) ? "set" :
> "clear",
>> - local_db, binlog_filter->db_ok(local_db)));
>> DBUG_RETURN(0);
>> }
>> #endif /* HAVE_REPLICATION */
>> diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
>> --- a/sql/log_event.cc 2007-08-27 20:22:01 +02:00
>> +++ b/sql/log_event.cc 2007-10-05 18:16:01 +02:00
>> @@ -5697,7 +5697,9 @@ Rows_log_event::Rows_log_event(const cha
>> *description_event)
>> : Log_event(buf, description_event),
>> m_row_count(0),
>> +#ifndef MYSQL_CLIENT
>> m_table(NULL),
>> +#endif
>> m_rows_buf(0), m_rows_cur(0), m_rows_end(0),
>> m_curr_row(NULL), m_curr_row_end(NULL),
>> m_key(NULL)
>> @@ -6168,6 +6170,8 @@ int Rows_log_event::do_apply_event(RELAY
>> unpack_current_row(rli);
>>
>> // at this moment m_curr_row_end should be set
>> + DBUG_PRINT("debug", ("m_curr_row_end: 0x%lx; m_curr_row: 0x%lx;
> m_rows_end: 0x%lx",
>> + m_curr_row_end, m_curr_row, m_rows_end));
>> DBUG_ASSERT(error || m_curr_row_end != NULL);
>> DBUG_ASSERT(error || m_curr_row < m_curr_row_end);
>> DBUG_ASSERT(error || m_curr_row_end <= m_rows_end);
>> diff -Nrup a/sql/rpl_record.cc b/sql/rpl_record.cc
>> --- a/sql/rpl_record.cc 2007-08-26 14:31:03 +02:00
>> +++ b/sql/rpl_record.cc 2007-10-05 18:16:01 +02:00
>> @@ -65,6 +65,8 @@ pack_row(TABLE *table, MY_BITMAP const*
>> my_ptrdiff_t const rec_offset= record - table->record[0];
>> my_ptrdiff_t const def_offset= table->s->default_values -
> table->record[0];
>>
>> + DBUG_ENTER("pack_row");
>> +
>> /*
>> We write the null bits and the packed records using one pass
>> through all the fields. The null bytes are written little-endian,
>> @@ -96,26 +98,11 @@ pack_row(TABLE *table, MY_BITMAP const*
>> For big-endian machines, we have to make sure that the
>> length is stored in little-endian format, since this is the
>> format used for the binlog.
>> -
>> - We do this by setting the db_low_byte_first, which is used
>> - inside some store_length() to decide what order to write the
>> - bytes in.
>> -
>> - In reality, db_log_byte_first is only set for legacy table
>> - type Isam, but in the event of a bug, we need to guarantee
>> - the endianess when writing to the binlog.
>> -
>> - This is currently broken for NDB due to BUG#29549, so we
>> - will fix it when NDB has fixed their way of handling BLOBs.
>> */
>> -#if 0
>> - bool save= table->s->db_low_byte_first;
>> - table->s->db_low_byte_first= TRUE;
>> -#endif
>> - pack_ptr= field->pack(pack_ptr, field->ptr + offset);
>> -#if 0
>> - table->s->db_low_byte_first= save;
>> -#endif
>> + const uchar *old_pack_ptr= pack_ptr;
>> + pack_ptr= field->pack(pack_ptr, field->ptr + offset, UINT_MAX,
> TRUE);
>> + DBUG_PRINT("debug", ("field: %s; pack_ptr: 0x%lx; pack_ptr':0x%lx;
> bytes: %d",
>> + field->field_name, old_pack_ptr, pack_ptr,
> pack_ptr - old_pack_ptr));
>> }
>>
>> null_mask <<= 1;
>> @@ -143,8 +130,8 @@ pack_row(TABLE *table, MY_BITMAP const*
>> packed data. If it doesn't, something is very wrong.
>> */
>> DBUG_ASSERT(null_ptr == row_data + null_byte_count);
>> -
>> - return static_cast<size_t>(pack_ptr - row_data);
>> + DBUG_DUMP("row_data", row_data, pack_ptr - row_data);
>> + DBUG_RETURN(static_cast<size_t>(pack_ptr - row_data));
>> }
>> #endif
>>
>> @@ -242,18 +229,11 @@ unpack_row(RELAY_LOG_INFO const *rli,
>> Use the master's size information if available else call
>> normal unpack operation.
>> */
>> -#if 0
>> - bool save= table->s->db_low_byte_first;
>> - table->s->db_low_byte_first= TRUE;
>> -#endif
>> uint16 const metadata= tabledef->field_metadata(i);
>> - if (tabledef && metadata)
>> - pack_ptr= f->unpack(f->ptr, pack_ptr, metadata);
>> - else
>> - pack_ptr= f->unpack(f->ptr, pack_ptr);
>> -#if 0
>> - table->s->db_low_byte_first= save;
>> -#endif
>> + uchar const *const old_pack_ptr= pack_ptr;
>> + pack_ptr= f->unpack(f->ptr, pack_ptr, metadata, TRUE);
>> + DBUG_PRINT("debug", ("field: %s; metadata: 0x%x; pack_ptr: 0x%lx; pack_ptr':
> 0x%lx; bytes: %d",
>> + f->field_name, metadata, old_pack_ptr, pack_ptr,
> pack_ptr - old_pack_ptr));
>> }
>>
>> null_mask <<= 1;
>> @@ -265,6 +245,7 @@ unpack_row(RELAY_LOG_INFO const *rli,
>> throw away master's extra fields
>> */
>> uint max_cols= min(tabledef->size(), cols->n_bits);
>> + DBUG_PRINT("debug", ("Master has %u fields, slave %u", tabledef->size(),
> cols->n_bits));
>> for (; i < max_cols; i++)
>> {
>> if (bitmap_is_set(cols, i))
>> @@ -279,6 +260,7 @@ unpack_row(RELAY_LOG_INFO const *rli,
>>
>> if (!((null_bits & null_mask) && tabledef->maybe_null(i)))
>> pack_ptr+= tabledef->calc_field_size(i, (uchar *) pack_ptr);
>> + DBUG_PRINT("debug", ("pack_ptr: 0x%lx", pack_ptr));
>> null_mask <<= 1;
>> }
>> }
>> @@ -288,6 +270,8 @@ unpack_row(RELAY_LOG_INFO const *rli,
>> really wrong.
>> */
>> DBUG_ASSERT(null_ptr == row_data + master_null_byte_count);
>> +
>> + DBUG_DUMP("row_data", row_data, pack_ptr - row_data);
>>
>> *row_end = pack_ptr;
>> if (master_reclength)
>> diff -Nrup a/sql/sql_show.cc b/sql/sql_show.cc
>> --- a/sql/sql_show.cc 2007-08-16 18:04:11 +02:00
>> +++ b/sql/sql_show.cc 2007-10-05 18:16:02 +02:00
>> @@ -29,6 +29,8 @@
>> #include "event_data_objects.h"
>> #include <my_dir.h>
>>
>> +#define STR_OR_NIL(S) ((S) ? (S) : "<nil>")
>> +
>> #ifdef WITH_PARTITION_STORAGE_ENGINE
>> #include "ha_partition.h"
>> #endif
>> @@ -3096,8 +3098,8 @@ int get_all_tables(THD *thd, TABLE_LIST
>> schema_table_idx= get_schema_table_idx(schema_table);
>> get_lookup_field_values(thd, cond, tables, &lookup_field_vals);
>> DBUG_PRINT("INDEX VALUES",("db_name='%s', table_name='%s'",
>> - lookup_field_vals.db_value.str,
>> - lookup_field_vals.table_value.str));
>> + STR_OR_NIL(lookup_field_vals.db_value.str),
>> + STR_OR_NIL(lookup_field_vals.table_value.str)));
>>
>> if (!lookup_field_vals.wild_db_value &&
> !lookup_field_vals.wild_table_value)
>> {
>>
>> --
>> MySQL Code Commits Mailing List
>> For list archives: http://lists.mysql.com/commits
>> To unsubscribe: http://lists.mysql.com/commits?unsub=1
>>
>>
>
> regards,
>
> Andrei
>
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
