List:Commits« Previous MessageNext Message »
From:Mats Kindahl Date:July 25 2007 8:45am
Subject:Re: bk commit into 5.1 tree (cbell:1.2564)
View as plain text  
Hi Chuck!

I see several patches for this worklog, so I'm just reviewing the last 
one (which you also references in the private mail).

Some general advice:

Try to avoid or group dynamic allocations of memory whenever possible. 
Each memory allocation takes a mutex to prevent corruption of the heap 
memory, which can cause threads to caravan if there are too many 
allocations. It is often better to speculatively allocate memory that 
might not be needed just to avoid waiting on the mutex.

It is not necessary to use "short int": using "short" will suffice.

The code is generally using my_malloc() and my_multi_malloc() instead of 
"new" when allocating memory for raw buffers. Using "new" will, however, 
call my_malloc() internally, so there is no problem with using "new": 
this is just a style issue.

Please read the comments below, there are some issues that needs closer 
inspection.

Just my few cents,
Mats Kindahl


cbell@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.1 repository of cbell. When cbell 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-07-23 17:19:19-04:00, cbell@mysql_cab_desk. +23 -0
>   WL#3228 (NDB) : RBR using different table defs on slave/master
>   
>   This patch adds the ability to store extra field metadata in the table
>   map event. This data can include pack_length() or field_lenght() for
>   fields such as CHAR or VARCHAR enabling developers to add code that
>   can check for compatibilty between master and slave columns. More 
>   importantly, the extra field metadata can be used to store data from the
>   master correctly should a VARCHAR field on the master be <= 255 bytes 
>   while the same field on the slave is > 255 bytes. '
>   
>   The patch also includes the needed changes to unpack to ensure that data
>   which is smaller on the master can be unpacked correctly on the slave.
>
>   mysql-test/r/rpl_colSize.result@stripped, 2007-07-23 17:19:14-04:00,
> cbell@mysql_cab_desk. +47 -0
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>     
>
>   mysql-test/r/rpl_colSize.result@stripped, 2007-07-23 17:19:14-04:00,
> cbell@mysql_cab_desk. +0 -0
>
>   mysql-test/r/rpl_ndb_log.result@stripped, 2007-07-23 17:19:01-04:00,
> cbell@mysql_cab_desk. +4 -4
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_basic_11bugs.result@stripped, 2007-07-23 17:19:02-04:00,
> cbell@mysql_cab_desk. +2 -2
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_create_table.result@stripped, 2007-07-23 17:19:02-04:00,
> cbell@mysql_cab_desk. +45 -42
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_flsh_tbls.result@stripped, 2007-07-23 17:19:03-04:00,
> cbell@mysql_cab_desk. +2 -2
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_inexist_tbl.result@stripped, 2007-07-23 17:19:04-04:00,
> cbell@mysql_cab_desk. +1 -1
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_log.result@stripped, 2007-07-23 17:19:04-04:00,
> cbell@mysql_cab_desk. +5 -5
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_log_innodb.result@stripped, 2007-07-23 17:19:05-04:00,
> cbell@mysql_cab_desk. +5 -5
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_max_relay_size.result@stripped, 2007-07-23 17:19:05-04:00,
> cbell@mysql_cab_desk. +10 -10
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_row_until.result@stripped, 2007-07-23 17:19:06-04:00,
> cbell@mysql_cab_desk. +4 -4
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/r/rpl_truncate_7ndb.result@stripped, 2007-07-23 17:19:07-04:00,
> cbell@mysql_cab_desk. +32 -32
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a result file for the new test designed to test the 
>     feature of having columns on the master that are smaller than what is 
>     on the slave. 
>
>   mysql-test/t/binlog_row_mix_innodb_myisam.test@stripped, 2007-07-23 17:19:07-04:00,
> cbell@mysql_cab_desk. +1 -1
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch changes the test to coincide with changes to binlog
>     size of table map event.
>
>   mysql-test/t/rpl_colSize.test@stripped, 2007-07-23 17:19:14-04:00, cbell@mysql_cab_desk.
> +111 -0
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch contains a new test designed to test the feature of having
>     columns on the master that are smaller than what is on the slave. 
>     
>
>   mysql-test/t/rpl_colSize.test@stripped, 2007-07-23 17:19:14-04:00, cbell@mysql_cab_desk.
> +0 -0
>
>   mysql-test/t/rpl_row_create_table.test@stripped, 2007-07-23 17:19:08-04:00,
> cbell@mysql_cab_desk. +5 -5
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch changes the test to coincide with changes to binlog
>     size of table map event.
>
>   mysql-test/t/rpl_row_flsh_tbls.test@stripped, 2007-07-23 17:19:08-04:00,
> cbell@mysql_cab_desk. +1 -1
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch changes the test to coincide with changes to binlog
>     size of table map event.
>
>   sql/field.cc@stripped, 2007-07-23 17:19:09-04:00, cbell@mysql_cab_desk. +132 -0
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch includes updates to the unpack() methods for the variable
>     length fields. A new parameter was added (from_length) that is the
>     value stored in the field_metadata of the table map from the table_def
>     class. If the value is non-zero and less than what the field on the 
>     slave is then use the from_length else use the original value from the
>     field on the slave.
>
>   sql/field.h@stripped, 2007-07-23 17:19:10-04:00, cbell@mysql_cab_desk. +42 -0
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch includes updates to the unpack() methods for the variable
>     length fields. A new parameter was added (from_length) that is the
>     value stored in the field_metadata of the table map from the table_def
>     class. 
>
>   sql/log_event.cc@stripped, 2007-07-23 17:19:10-04:00, cbell@mysql_cab_desk. +210 -8
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch adds methods to calculate the field metadata size, prepare
>     the field metadata for writing to the binlog, and additions to the
>     Table_map_log_event::write_body method to include the field metadata 
>     in the table map that is written to the binlog.
>
>   sql/log_event.h@stripped, 2007-07-23 17:19:11-04:00, cbell@mysql_cab_desk. +10 -0
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch adds method declarations and variables needed to support
>     storing field metadata in the table map that is written to the binlog.
>
>   sql/rpl_record.cc@stripped, 2007-07-23 17:19:11-04:00, cbell@mysql_cab_desk. +14 -4
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch modifies the unpack_row() method to unpack fields passing in
>     the value from the table_def class. This value is the extra field
>     metadata stored there from the master.
>
>   sql/rpl_rli.h@stripped, 2007-07-23 17:19:12-04:00, cbell@mysql_cab_desk. +10 -0
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch adds a helper function to retrieve the table_def for a given
>     table in the RPL_TABLE_LIST structure.
>
>   sql/rpl_utility.cc@stripped, 2007-07-23 17:19:13-04:00, cbell@mysql_cab_desk. +55 -35
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch adds a helper method that retrieves the correct size 
>     parameter for the field. This method is used to compare the size as
>     sent by the master with that on the slave for all types of fields that
>     can vary in size and storage requirements. 
>
>   sql/rpl_utility.h@stripped, 2007-07-23 17:19:13-04:00, cbell@mysql_cab_desk. +125 -5
>     WL#3228 : RBR using different table defs on slave/master
>     
>     This patch changes the table_def class constructor to pass in the raw
>     data read from the table map and extract it into an array of dimension
>     size (number of fields). It also adds a method to return the field 
>     metadata for any field. The method returns the data stored in the table
>     map or 0 if no data was stored for that field. Lastly, a method to return
>     the results of field->maybe_null() is included so that the slave can
>     determine if a field that is not on the slave is null.
>
> # This is a BitKeeper patch.  What follows are the unified diffs for the
> # set of deltas contained in the patch.  The rest of the patch, the part
> # that BitKeeper cares about, is below these diffs.
> # User:	cbell
> # Host:	mysql_cab_desk.
> # Root:	C:/source/c++/mysql-5.1_WL_3228_ndb
>
> --- 1.393/sql/field.cc	2007-07-23 17:19:38 -04:00
> +++ 1.394/sql/field.cc	2007-07-23 17:19:38 -04:00
> @@ -2635,6 +2635,41 @@
>            (new_field->decimals == dec));
>  }
>  
> +/*
> +  This method is used to unpack a decimal or numeric field from a master
> +  whose size of the field is less than that of the slave.
> +*/
>   

Since you are adding a function, you could just as well comment the 
function using Doxygen comments. Please make sure to have a description 
for each parameter and the return value and indicate what values are 
allowed (e.g., can from_length be zero and what does that mean). Since 
this is a virtual function, it might be better to write the Doxygen 
comment for the top-most (in the inheritance hierarchy) version of the 
function.

Since the from_length is just a piece of data representing the 
parameters to the field, I would suggest another name ("param_data"? 
"params"?). I would also suggest that you for each function that 
interprets this piece of data (e.g., this one), you describe in the 
comment how it is interpreted.

> +const char *Field_new_decimal::unpack(char* to, 
> +                                      const char *from, 
> +                                      uint from_length)
> +{
> +  uint from_precision= (from_length & 0xff00) >> 8U;
> +  uint from_decimal= from_length & 0x00ff;
> +  uint length=pack_length();
> +  uint from_pack_len= my_decimal_get_binary_size(from_precision, from_decimal);
> +  uint len= (from_length && (from_pack_len < length)) ?
> +            from_pack_len : length;
>   

Can from_length be 0 for a DECIMAL? I don't think it can (after 
checking, I'm sure it can't be), so the check for "from_length" in the 
conjunction above is unnecessary (but an assertion does not hurt here).

> +  if (from_pack_len && (from_pack_len < length))
>   

Can from_pack_len be zero (I'm very sure it can't, check inside 
decimal_bin_size())? Otherwise, the check is unnecessary.

> +  {
> +    /*
> +      If the master's data is smaller than the slave, we need to convert
> +      the binary to decimal then resize the decimal converting it back to
> +      a decimal and write that to the raw data buffer.
> +    */
> +    decimal_digit_t dec_buf[DECIMAL_MAX_PRECISION];
> +    decimal_t dec;
> +    dec.len= from_precision;
> +    dec.buf= dec_buf;
> +    double dbl;
> +    bin2decimal((char *)from, &dec, from_precision, from_decimal);
>   
Please add a comment mentioning that bin2decimal does not change dec.len 
(if that is true, which is seems to be from reading the code). This is 
easy to assume otherwise. Otherwise, this resizing would be a good 
addition to decimal.c since it is feasible that it will be used elsewhere

> +    decimal2double(&dec, &dbl);
> +    double2decimal(dbl, &dec);
>   

These two statements are very slow: the latter one is converting the 
double to a string before actually converting it to a decimal. I don't 
know if it is possible to convert them by working with the decimal_t 
buffer (it seems feasible), but that would definitely speed things up.

> +    decimal2bin(&dec, to, precision, decimals());
> +  }
> +  else
> +    memcpy(to, from, len); // Sizes are the same, just copy the data.
> +  return from+len;
> +}
>  
>  /****************************************************************************
>  ** tiny int
> @@ -6294,6 +6329,30 @@
>  }
>  
>  
> +/*
> +  When unpacking data that may be smaller in the "from" than the "to," use
> +  the from_length to unpack rather than the to's length.
> +*/
> +const char *Field_string::unpack(char *to, const char *from, uint from_length)
> +{
> +  uint from_len= (from_length & 0xff00) >> 8U;    // real_type.
> +  from_len= from_length & 0x00ff;                 // length.
>   

Here you overwrite the value of from_len with the lower half of 
from_length: is this really intentional? Note: from_len is now limited 
to the range [0,255], but both CHAR and VARCHAR can have a parameter in 
the range [0,2^16). I also suggest adding const qualifiers to values 
that you don't intend to change: it is easier to catch this kind of 
mistakes that way; i.e.:

uint const from_len= ...;

> +  uint length= 0;
> +  uint f_length= (from_length && (from_len < field_length)) ? 
> +                from_len : field_length;
> +  if (f_length > 255)
>   

Umh... when field_length is > 255, it can never be the case that 
from_len < field_length is false since from_len is bounded to the range 
[0,255]. This means that if the field length on the master is larger 
than 255 and the field length on the slave is the same as on the master, 
the expression from_len < field_length is false even though the 
field_length on the master and slave are identical.

> +  {
> +    length= uint2korr(from);
> +    from+= 2;
> +  }
> +  else
> +    length= (uint) *from++;
> +  memcpy(to, from, (int) length);
> +  bfill(to+length, field_length - length, ' ');
>   

Isn't there a fill function for CHAR and VARCHAR fields? Hardcoding it 
to space introduce the possibility of duplicating code, which in turn 
can introduce subtle bugs. Indeed, in the Field_string::store() 
function, there is the following code to actually perform the padding:

  if (copy_length < field_length)
    field_charset->cset->fill(field_charset,(char*) ptr+copy_length,
                              field_length-copy_length,
                              field_charset->pad_char);

I actually think that you can use the Field_string::store() function 
instead of rolling your own version as you've done above.

> +  return from+length;
> +}
> +
> +
>  const char *Field_string::unpack(char *to, const char *from)
>  {
>    uint length;
> @@ -6783,6 +6842,36 @@
>  
>  
>  /*
> +  When unpacking data that may be smaller in the "from" than the "to," use
> +  the from_length to unpack rather than the to's length_bytes.
> +*/
> +const char *Field_varstring::unpack(char *to, const char *from, uint from_length)
> +{
> +  uint length;
> +  uint l_bytes= (from_length && (from_length < field_length)) ? 
> +                (from_length <= 255) ? 1 : 2 : length_bytes;
> +  if (l_bytes == 1)
> +    if (length_bytes == 1)
> +      length= (uint) (*to= *from++);
> +    else
> +    {
> +      to[1]= 0;
> +      to[0]= *from++;
> +      length= to[0];
> +    }
> +  else
> +  {
> +    length= uint2korr(from);
> +    to[0]= *from++;
> +    to[1]= *from++;
> +  }
> +  if (length)
> +    memcpy(to+ length_bytes, from, length);
> +  return from+length;
> +}
> +
> +
> +/*
>   

I think you can use Field_varstring::store() here as well, similar to above.

>    unpack field packed with Field_varstring::pack()
>  */
>  
> @@ -7473,6 +7562,16 @@
>  }
>  
>  
> +/*
> +  Note: from_length included to satisfy inheritance rules, but is not needed
> +  for blob fields.
> +*/
> +const char *Field_blob::unpack(char *to, const char *from, uint from_length)
> +{
> +  return unpack(to, from);
> +}
>   

Are you storing the from_length (or param_data, or whatever it is 
called) for BLOB fields or are you optimizing it away? My personal 
opinion is that you should keep simple (i.e., linear) relations between 
positions in the fields since that allow the compiler to generate more 
efficient code, e.g., something like "param_data = 
param_array[column_pos]". See below for an example.

> +
> +
>  const char *Field_blob::unpack(char *to, const char *from)
>  {
>    memcpy(to,from,packlength);
> @@ -8507,6 +8606,39 @@
>    length= min(bytes_in_rec, max_length - (bit_len > 0));
>    memcpy(to, from, length);
>    return to + length;
> +}
> +
> +
> +/*
> +  When unpacking data that may be smaller in the "from" than the "to," use
> +  the from_length to unpack rather than the to's length.
> +*/
> +const char *Field_bit::unpack(char *to, const char *from, uint from_length)
> +{
> +  uint from_bit_len= (from_length & 0xff00) >> 8U;
> +  uint from_len= from_length & 0x00ff;
> +  /*
> +    If the master and slave have the same sizes, then use the old
> +    unpack() method.
> +  */
> +  uint master_width= from_len + ((from_bit_len > 0) ? 1 : 0);
> +  uint slave_width= bytes_in_rec + ((bit_len > 0) ? 1 : 0);
> +  if (master_width == slave_width) 
> +    return(unpack(to, from));
> +  if (from_bit_len > 0)
>   

Can n be zero in a BIT(n) field? According to the manual, it can't. No 
harm in replacing this if with an assertion though.

> +  {
> +    /*
> +      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, from_bit_len);
> +    from++;
> +  }
> +  memcpy(to, from, from_len);
> +  return from + from_len;
>   

Check if you can use Field_bit::store() here as well. Might not be the 
case, but code duplication is a bad thing and should be avoided as much 
as possible.

>  }
>  
>  
>
> --- 1.226/sql/field.h	2007-07-23 17:19:38 -04:00
> +++ 1.227/sql/field.h	2007-07-23 17:19:38 -04:00
> @@ -342,6 +342,36 @@
>      memcpy(to,from,length);
>      return to+length;
>    }
> +  virtual const char *unpack(char* to, const char *from, uint from_length)
> +  {
> +    uint length=pack_length();
> +    int from_type= 0;
> +    /*
> +      If from length is > 255, it has encoded data in the upper bits. Need
> +      to mask it out.
> +    */
> +    if (from_length > 255)
> +    {
> +      from_type= (from_length & 0xff00) >> 8U;  // real_type.
> +      from_length= from_length & 0x00ff;        // length.
> +    }
> +    uint len= (from_length && (from_length < length)) ?
> +              from_length : length;
> +    /*
> +      If the length is the same, use old unpack method.
> +      If the from_length is 0, use the old unpack method.
>   

What does it mean that the from_length is 0?

> +      If the real_types are not the same, use the old unpack method.
>   

When can this occur? It seems strange that this would be allowed, but it 
is possible.

> +    */
> +    if ((length == from_length) ||
> +        (from_length == 0) ||
> +        (from_type != real_type()))
> +      return(unpack(to, from));
> +    if (from_length > length)
> +      memcpy(to,from,length);
> +    else
> +      memcpy(to,from,len);
>   

memcpy(to, from, from_length > length ? length : len) is an alternative. 
Makes no difference in the code, however, since the expression is 
usually rewritten to an if-statement internally.


> +    return from+len;
> +  }
>   

This function is long enough to warrant putting into the .cc file: there 
are no advantages to keeping it in the header file and there are several 
disadvantages.

>    virtual const char *unpack(char* to, const char *from)
>    {
>      uint length=pack_length();
> @@ -613,6 +643,7 @@
>    uint size_of() const { return sizeof(*this); } 
>    uint32 pack_length() const { return (uint32) bin_size; }
>    uint is_equal(create_field *new_field);
> +  const char *unpack(char* to, const char *from, uint from_length);
>   

If this function is virtual, I suggest putting a "virtual" in the 
declaration, even in the case when it is implicitly virtual.

>  };
>  
>  
> @@ -1154,6 +1185,7 @@
>    void sort_string(char *buff,uint length);
>    void sql_type(String &str) const;
>    char *pack(char *to, const char *from, uint max_length=~(uint) 0);
> +  const char *unpack(char* to, const char *from, uint from_length);
>   

See above.

>    const char *unpack(char* to, const char *from);
>    int pack_cmp(const char *a,const char *b,uint key_length,
>                 my_bool insert_or_update);
> @@ -1224,6 +1256,7 @@
>    char *pack(char *to, const char *from, uint max_length=~(uint) 0);
>    char *pack_key(char *to, const char *from, uint max_length);
>    char *pack_key_from_key_image(char* to, const char *from, uint max_length);
> +  const char *unpack(char* to, const char *from, uint from_length);
>    const char *unpack(char* to, const char *from);
>    const char *unpack_key(char* to, const char *from, uint max_length);
>    int pack_cmp(const char *a, const char *b, uint key_length,
> @@ -1279,6 +1312,9 @@
>                    l_char_length <= 16777215 ? 3 : 4;
>      }
>    }
> +  Field_blob(uint32 packlength_arg)
> +    :Field_longstr((char*) 0, 0, (uchar*) "", 0, NONE, "temp",
> system_charset_info),
> +    packlength(packlength_arg) {}
>    enum_field_types type() const { return MYSQL_TYPE_BLOB;}
>    enum ha_base_keytype key_type() const
>      { return binary() ? HA_KEYTYPE_VARBINARY2 : HA_KEYTYPE_VARTEXT2; }
> @@ -1300,6 +1336,8 @@
>    void sort_string(char *buff,uint length);
>    uint32 pack_length() const
>    { return (uint32) (packlength+table->s->blob_ptr_size); }
> +  uint32 pack_length_no_ptr() const
> +  { return (uint32) (packlength); }
>   

Cast is not needed here since it is implicit anyway. Is there a specific 
reason to why the cast is there?

What does the function return? Since you are introducing a new function, 
I suggest adding a Doxygen comment describing what it does (if it is a 
virtual function, only documenting the top-most version is necessary).

>    uint32 sort_length() const;
>    inline uint32 max_data_length() const
>    {
> @@ -1315,6 +1353,8 @@
>    {
>      store_length(ptr, packlength, number);
>    }
> +  uint32 get_packed_size(const uchar *ptr)
> +    {return packlength + get_length((uint)ptr);}
>   

Function documentation here as well.

>  
>    inline uint32 get_length(uint row_offset=0)
>    { return get_length(ptr+row_offset); }
> @@ -1360,6 +1400,7 @@
>    char *pack(char *to, const char *from, uint max_length= ~(uint) 0);
>    char *pack_key(char *to, const char *from, uint max_length);
>    char *pack_key_from_key_image(char* to, const char *from, uint max_length);
> +  const char *unpack(char *to, const char *from, uint from_length);
>   

virtual

>    const char *unpack(char *to, const char *from);
>    const char *unpack_key(char* to, const char *from, uint max_length);
>    int pack_cmp(const char *a, const char *b, uint key_length,
> @@ -1534,6 +1575,7 @@
>    uint32 pack_length_in_rec() const { return bytes_in_rec; }
>    void sql_type(String &str) const;
>    char *pack(char *to, const char *from, uint max_length=~(uint) 0);
> +  const char *unpack(char *to, const char *from, uint from_length);
>   

virtual

>    const char *unpack(char* to, const char *from);
>    virtual void set_default();
>  
>
> --- 1.279/sql/log_event.cc	2007-07-23 17:19:38 -04:00
> +++ 1.280/sql/log_event.cc	2007-07-23 17:19:38 -04:00
> @@ -5976,11 +5976,8 @@
>  #ifdef HAVE_QUERY_CACHE
>      query_cache.invalidate_locked_for_write(rli->tables_to_lock);
>  #endif
> -    const_cast<RELAY_LOG_INFO*>(rli)->clear_tables_to_lock();
>    }
>  
> -  DBUG_ASSERT(rli->tables_to_lock == NULL && rli->tables_to_lock_count
> == 0);
>   

Is this part of the patch? Why are they removed? It looks more like a 
bugfix. (Just noted the code below, so you can ignore this comment).

> -
>    TABLE* table=
> const_cast<RELAY_LOG_INFO*>(rli)->m_table_map.get_table(m_table_id);
>  
>    if (table)
> @@ -6076,6 +6073,13 @@
>      }
>    }
>  
> +  /*
> +    We need to delay this clear until the table def is no longer needed.
> +    The table def is needed in unpack_row().
> +  */
> +  if (rli->tables_to_lock && get_flags(STMT_END_F))
> +    const_cast<RELAY_LOG_INFO*>(rli)->clear_tables_to_lock();
>   

Ah! IC. But why do you need to keep the table map events around? Isn't 
that what table_def is to be used for? (I can see reasons to keeping the 
table maps around, but why are *you* doing it.)

> +
>    if (error)
>    {                     /* error has occured during the transaction */
>      slave_print_msg(ERROR_LEVEL, rli, thd->net.last_errno,
> @@ -6314,6 +6318,147 @@
>  	Table_map_log_event member functions and support functions
>  **************************************************************************/
>  
> +/**
> +  * Calculate field metadata size based on the real_type of the field.
> +  *
> +  * @returns int Size of field metadata.
> +  */
>   

As I said before: I don't see a big advantage to keeping different-sized 
data for different fields, especially since you only save one byte for 
BLOBs (and for IEEE numbers, but they are not that common in a 
database), which are usually large. The advantages to keeping a constant 
size for each fields improves performance significantly (provided the 
code is written correctly).

> +#if !defined(MYSQL_CLIENT)
> +int Table_map_log_event::get_field_metadata_size()
> +{
> +  DBUG_ENTER("Table_map_log_event::get_field_metadata_size");
> +  int size= 0;
> +  for (unsigned int i= 0 ; i < m_table->s->fields ; i++)
> +  {
> +    switch (m_table->s->field[i]->real_type()) {
> +    case MYSQL_TYPE_TINY_BLOB:
> +    case MYSQL_TYPE_BLOB:
> +    case MYSQL_TYPE_MEDIUM_BLOB:
> +    case MYSQL_TYPE_LONG_BLOB:
> +    case MYSQL_TYPE_DOUBLE:
> +    case MYSQL_TYPE_FLOAT:
> +    {
> +      size++;                         // Store one byte here.
> +      break; 
> +    }
> +    case MYSQL_TYPE_BIT:
> +    case MYSQL_TYPE_NEWDECIMAL:
> +    case MYSQL_TYPE_ENUM:
> +    case MYSQL_TYPE_STRING:
> +    case MYSQL_TYPE_VARCHAR:
> +    case MYSQL_TYPE_SET:
> +    {
> +      size= size + sizeof(short int); // Store short int here.
> +      break;
> +    }
> +    default:
> +      break;
> +    }
> +  }
> +  m_field_metadata_size= size;
> +  DBUG_PRINT("info", ("Table_map_log_event: %d bytes in field metadata.",
> +                       (int)m_field_metadata_size));
> +  DBUG_RETURN(m_field_metadata_size);
> +}
> +#endif /* !defined(MYSQL_CLIENT) */
> +
> +/**
> +  * How replication of field metadata works.
>   
I think you should call it field type parameters instead.

Use the Doxygen construct:

@page How replication of field metadata works.

To get a separate page about the stuff. We need to consider how to 
structure the documentation for replication later (post-5.1 GA), so this 
is not important right now. We probably need to create a separate module 
(in the Doxygen sense) with all the replication stuff in it, but since 
there are bits and pieces for replication everywhere, we need to think 
about how we structure the stuff.

> +  * 
> +  * When a table map is created, the master first calls 
> +  * Table_map_log_event::get_field_metadata_size() which calculates how many 
> +  * values will be in the field metadata. Only those fields that require the 
> +  * extra data are added (see table above). The master then loops through all
> +  * of the fields in the table calling the method 
> +  * Table_map_log_event::get_field_metadata() which returns the values for the 
> +  * field that will be saved in the metadata and replicated to the slave. Once 
> +  * all fields have been processed, the table map is written to the binlog 
> +  * adding the size of the field metadata and the field metadata to the end of 
> +  * the body of the table map.
> +  * 
> +  * When a table map is read on the slave, the field metadata is read from the 
> +  * table map and passed to the table_def class constructor which saves the 
> +  * field metadata from the table map into an array based on the type of the 
> +  * field. Field metadata values not present (those fields that do not use extra 
> +  * data) in the table map are initialized as zero (0). The array size is the 
> +  * same as the columns for the table on the slave.
> +*/
> +
> +/**
> +  * Save the field metadata based on the real_type of the field.
> +  * The metadata saved depends on the type of the field. Some fields
> +  * store a single byte for pack_length() while others store two bytes
> +  * for field_length (max length).
> +  *
> +  * @returns int 0 = Ok.
>   
@retval 0 OK

> +  */
> +#if !defined(MYSQL_CLIENT)
> +int Table_map_log_event::get_field_metadata()
>   

"get" is a verb usually used for non-mutating member functions: I would 
suggest using "retrieve", "save", or "store" instead to avoid confusion.

> +{
> +  DBUG_ENTER("Table_map_log_event::get_field_metadata");
> +  int index= 0;
> +  for (unsigned int i= 0 ; i < m_table->s->fields ; i++)
> +  {
> +    switch (m_table->s->field[i]->real_type()) {
>   

Use a virtual function instead of a switch to get the metadata: this is 
what virtual functions are to be used for. You have two alternatives:

  for (unsigned int i= 0 ; i < m_table->s->fields ; i++)
    m_field_metadata[index++]= m_table->s->field[i]->get_metadata();

or

  uint16 ptr_meta= m_field_metadata;
  for (unsigned int i= 0 ; i < m_table->s->fields ; i++)
    ptr_meta = m_table->s->field[i]->get_metadata(ptr_meta);

If you always keep two bytes for the metadata/parameter data (as I 
suggested above), the following loop is the one that the compiler can 
work best with:

   for (unsigned int i= 0 ; i < m_table->s->fields ; i++)
    m_field_metadata[i]= m_table->s->field[i]->get_metadata();

Since there is a linear relation between the positions in the metadata 
and the positions in the field, alias analysis is possible (even simple) 
and the loop can be reversed, unrolled, pipelined, and a myriad of other 
optimizations are available.

> +    case MYSQL_TYPE_NEWDECIMAL:
> +    {
> +      m_field_metadata[index++]= 
> +        (uchar)((Field_new_decimal *)m_table->s->field[i])->precision;
> +      m_field_metadata[index++]= 
> +        (uchar)((Field_new_decimal *)m_table->s->field[i])->decimals();
> +      break;
> +    }
> +    case MYSQL_TYPE_TINY_BLOB:
> +    case MYSQL_TYPE_BLOB:
> +    case MYSQL_TYPE_MEDIUM_BLOB:
> +    case MYSQL_TYPE_LONG_BLOB:
> +    {
> +      m_field_metadata[index++]= 
> +       (uchar)((Field_blob *)m_table->s->field[i])->pack_length_no_ptr();
> +      break;
> +    }
> +    case MYSQL_TYPE_DOUBLE:
> +    case MYSQL_TYPE_FLOAT:
> +    {
> +      m_field_metadata[index++]=
> (uchar)m_table->s->field[i]->pack_length();
> +      break;
> +    }
> +    case MYSQL_TYPE_BIT:
> +    { 
> +      m_field_metadata[index++]= 
> +        (uchar)((Field_bit *)m_table->s->field[i])->bit_len;
> +      m_field_metadata[index++]= 
> +        (uchar)((Field_bit *)m_table->s->field[i])->bytes_in_rec;
> +      break;
>   

Is this code really correct? There are some discrepancies between this 
code and the table_def::get_field_size() below. AFAICT, this code saves 
the data as:

    |---- 16 bits ----|---- 16 bits ----|
    |     bit_len     |   bytes_in_rec  |

while the code below (in get_field_size()) reads the field size as if it 
was:

    |----- 8 bits ----|----- 8 bits ----|
    |     bit_len     |   bytes_in_rec  |

After reading a little closer, I see that 
Table_map_log_event::m_field_metadata and table_def::m_field_metadata 
are "false friends" in that they have different types. Please change 
either the name of one of the fields, or use the same type for both 
fields to avoid confusion. As a side note: if you switch to the 
suggestion I had above (using virtual functions instead of a switch), 
the two fields will have the same type.

> +    }
> +    case MYSQL_TYPE_VARCHAR:
> +    {
> +      short int *x= (short int *)&m_field_metadata[index];
> +      int2store(x, m_table->s->field[i]->field_length);
> +      index= index + sizeof(short int);
> +      break;
> +    }
> +    case MYSQL_TYPE_STRING:
> +    {
> +      m_field_metadata[index++]= (uchar)m_table->s->field[i]->real_type();
> +      m_field_metadata[index++]= m_table->s->field[i]->field_length;
> +      break;
> +    }
> +    case MYSQL_TYPE_ENUM:
> +    case MYSQL_TYPE_SET:
> +    {
> +      m_field_metadata[index++]= (uchar)m_table->s->field[i]->real_type();
> +      m_field_metadata[index++]= m_table->s->field[i]->pack_length();
> +      break;
> +    }
> +    default:
> +      break;
> +    }
> +  }
> +  DBUG_RETURN(0);
> +}
> +#endif /* !defined(MYSQL_CLIENT) */
> +
>  /*
>    Constructor used to build an event for writing to the binary log.
>    Mats says tbl->s lives longer than this event so it's ok to copy pointers
> @@ -6328,8 +6473,8 @@
>      m_dblen(m_dbnam ? tbl->s->db.length : 0),
>      m_tblnam(tbl->s->table_name.str),
>      m_tbllen(tbl->s->table_name.length),
> -    m_colcnt(tbl->s->fields), m_coltype(0),
> -    m_table_id(tid),
> +    m_colcnt(tbl->s->fields), m_field_metadata(0),
> +    m_table_id(tid), m_null_bits(0), m_num_null_bytes(0),
>      m_flags(flags)
>  {
>    DBUG_ASSERT(m_table_id != ~0UL);
> @@ -6349,6 +6494,16 @@
>    m_data_size+= m_dblen + 2;	// Include length and terminating \0
>    m_data_size+= m_tbllen + 2;	// Include length and terminating \0
>    m_data_size+= 1 + m_colcnt;	// COLCNT and column types
> +  m_field_metadata_size= get_field_metadata_size();
> +
> +  /*
> +    Now set the size of the data to the size of the field metadata array
> +    plus one or two bytes for number of elements in the field metadata array.
> +  */
> +  if (m_field_metadata_size > 255)
> +    m_data_size+= m_field_metadata_size + 2; 
> +  else
> +    m_data_size+= m_field_metadata_size + 1; 
>  
>    /* If malloc fails, catched in is_valid() */
>    if ((m_memory= my_malloc(m_colcnt, MYF(MY_WME))))
> @@ -6357,6 +6512,26 @@
>      for (unsigned int i= 0 ; i < m_table->s->fields ; ++i)
>        m_coltype[i]= m_table->field[i]->type();
>    }
> +
> +  /*
> +    Calculate a bitmap for the results of maybe_null() for all columns.
> +    This will be writtento the table for use in skipping columns on the master
> +    that do not exist on the slave.
> +  */
>   

Since additional columns are always last in the row, why is this needed? 
I do agree that we need to store this field type information in the 
table map, but the above is not the reason. (What does "columns ... that 
do not exist" mean?)

> +  m_num_null_bytes= (m_table->s->fields + 7) / 8;
> +  m_data_size+= m_num_null_bytes;
> +  m_null_bits= new uchar[m_num_null_bytes];
> +  for (uint i = 0; i < m_num_null_bytes; i++)
> +    m_null_bits[i]= 0;
> +  for (unsigned int i= 0 ; i < m_table->s->fields ; ++i)
> +    if (m_table->field[i]->maybe_null())
> +      m_null_bits[(i / 8)]+= 1 << (i % 8);
>   

There is a MY_BITMAP with assorted functions to manipulate bitmaps. If 
you feel inclined, you can use that instead, but it is not necessary in 
this case since you are just generating data to save.

> +
> +  /*
> +    Create an array for the field metadata and store it.
> +  */
> +  m_field_metadata= new uchar[m_field_metadata_size];
>   

Use my_multi_malloc instead of new...

> +  get_field_metadata();
>  }
>  #endif /* !defined(MYSQL_CLIENT) */
>  
> @@ -6372,8 +6547,10 @@
>  #ifndef MYSQL_CLIENT
>    m_table(NULL),
>  #endif
> -  m_memory(NULL)
> +  m_memory(NULL),
> +  m_field_metadata(0), m_field_metadata_size(0)
>  {
> +  unsigned int bytes_read= 0;
>    DBUG_ENTER("Table_map_log_event::Table_map_log_event(const char*,uint,...)");
>  
>    uint8 common_header_len= description_event->common_header_len;
> @@ -6446,12 +6623,27 @@
>      memcpy(m_coltype, ptr_after_colcnt, m_colcnt);
>    }
>  
> +  ptr_after_colcnt= ptr_after_colcnt + m_colcnt;
> +  bytes_read= ptr_after_colcnt - (uchar *)buf;
> +  DBUG_PRINT("info", ("Bytes read: %d.\n", bytes_read));
> +  if (bytes_read < event_len)
> +  {
> +    m_field_metadata_size= net_field_length(&ptr_after_colcnt);
> +    m_field_metadata= new uchar[m_field_metadata_size];
> +    memcpy(m_field_metadata, ptr_after_colcnt, m_field_metadata_size);
> +    ptr_after_colcnt= (uchar*)ptr_after_colcnt + m_field_metadata_size;
> +    m_null_bits= new uchar[(m_colcnt + 7) / 8];
>   

Use my_multi_malloc above instead. Memory allocation is expensive in a 
multi-threaded environment.

> +    memcpy(m_null_bits, ptr_after_colcnt, (m_colcnt + 7) / 8);
> +  }
> +
>    DBUG_VOID_RETURN;
>  }
>  #endif
>  
>  Table_map_log_event::~Table_map_log_event()
>  {
> +  delete[] m_field_metadata;
> +  delete[] m_null_bits;
>    my_free(m_memory, MYF(MY_ALLOW_ZERO_PTR));
>  }
>  
> @@ -6585,7 +6777,8 @@
>        inside st_relay_log_info::clear_tables_to_lock() by calling the
>        table_def destructor explicitly.
>      */
> -    new (&table_list->m_tabledef) table_def(m_coltype, m_colcnt);
> +    new (&table_list->m_tabledef) table_def(m_coltype, m_colcnt, 
> +         m_field_metadata, m_field_metadata_size, m_null_bits);
>      table_list->m_tabledef_valid= TRUE;
>  
>      /*
> @@ -6644,13 +6837,22 @@
>    char *const cbuf_end= net_store_length((char*) cbuf, (uint) m_colcnt);
>    DBUG_ASSERT(static_cast<my_size_t>(cbuf_end - cbuf) <= sizeof(cbuf));
>  
> +  /*
> +    Store the size of the field metadata.
> +  */
> +  char mbuf[sizeof(m_field_metadata_size)];
> +  char *const mbuf_end= net_store_length(mbuf, (size_t) m_field_metadata_size);
> +
>    return (my_b_safe_write(file, dbuf,      sizeof(dbuf)) ||
>            my_b_safe_write(file, (const byte*)m_dbnam,   m_dblen+1) ||
>            my_b_safe_write(file, tbuf,      sizeof(tbuf)) ||
>            my_b_safe_write(file, (const byte*)m_tblnam,  m_tbllen+1) ||
>            my_b_safe_write(file, reinterpret_cast<byte*>(cbuf),
>                            cbuf_end - (char*) cbuf) ||
> -          my_b_safe_write(file, reinterpret_cast<byte*>(m_coltype),
> m_colcnt));
> +          my_b_safe_write(file, reinterpret_cast<byte*>(m_coltype), m_colcnt)
> ||
> +          my_b_safe_write(file, reinterpret_cast<byte*>(mbuf), (size_t)
> (mbuf_end - mbuf)) ||
> +          my_b_safe_write(file, reinterpret_cast<byte*>(m_field_metadata),
> m_field_metadata_size),
> +          my_b_safe_write(file, reinterpret_cast<byte*>(m_null_bits),
> m_num_null_bytes));
>   }
>  #endif
>  
>
> --- 1.149/sql/log_event.h	2007-07-23 17:19:38 -04:00
> +++ 1.150/sql/log_event.h	2007-07-23 17:19:38 -04:00
> @@ -2048,6 +2048,8 @@
>  
>    virtual int get_data_size() { return m_data_size; } 
>  #ifndef MYSQL_CLIENT
> +  virtual int get_field_metadata_size();
>   

Make it a const member function if it doesn't change data internally.

> +  virtual int get_field_metadata();
>    virtual bool write_data_header(IO_CACHE *file);
>    virtual bool write_data_body(IO_CACHE *file);
>    virtual const char *get_db() { return m_dbnam; }
> @@ -2083,6 +2085,14 @@
>    flag_set       m_flags;
>  
>    my_size_t      m_data_size;
> +
> +  uchar          *m_field_metadata;        // buffer for field metadata
> +  /*
> +    The size of field metadata buffer set by calling get_field_metadata_size()
> +  */
> +  ulong         m_field_metadata_size;   
> +  uchar        *m_null_bits;
> +  uint          m_num_null_bytes;
>  };
>  
>  
> --- New file ---
> +++ mysql-test/r/rpl_colSize.result	07/07/23 17:19:14
> stop slave;
> drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> reset master;
> reset slave;
> drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> start slave;
> DROP TABLE IF EXISTS t1;
> **** Testing WL#3228 changes. ****
> *** Create "wider" table on slave ***
> STOP SLAVE;
> RESET SLAVE;
> CREATE TABLE t1 (
> a float     (47),
> b double    (143,9),
> c decimal   (65,30),
> d numeric   (4,0),
> e bit       (32),
> f char      (21),
> g varchar   (1300),
> h binary    (33),
> j varbinary (200),
> k enum      ('5','6','7', '8','9','0'),
> l set      
> ('1','2','3','4','5','6','7','8','9','0','11','12','13','14','15','16','17','18','19','21','22','23','24','25','26','27','28','29'),
> m TINYBLOB,
> n BLOB,
> o MEDIUMBLOB,
> p LONGBLOB,
> q TINYTEXT,
> r TEXT,
> s MEDIUMTEXT,
> t LONGTEXT
> );
> *** Create same table on master but with narrow columns ***
> CREATE TABLE t1 (
> a float     (44),
> b double    (10,3),
> c decimal   (10,2),
> d numeric   (3,0),
> e bit       (16),
> f char      (10),
> g varchar   (100),
> h binary    (20),
> j varbinary (20),
> k enum      ('5','6','7'),
> l set       ('1','2','3','4','5','6','7','8','9','0'),
> m TINYBLOB,
> n BLOB,
> o MEDIUMBLOB,
> p LONGBLOB,
> q TINYTEXT,
> r TEXT,
> s MEDIUMTEXT,
> t LONGTEXT
> );
> RESET MASTER;
> *** Start replication ***
> START SLAVE;
> *** Insert data on master and display it. ***
> INSERT INTO t1 () VALUES (
> 17.567, 
> 2.123, 
> 10.20, 
> 125,
> hex(64),
> 'TEST',
> 'This is a test',
> 'binary data',
> 'more binary data',
> '6',
> '7',
> "blob 1",
> "blob  2",
> "blob   3",
> "blob    4",
> "text 1",
> "text  2",
> "text   3",
> "text    4");
> SELECT * FROM t1 ORDER BY a;
> a	b	c	d	e	f	g	h	j	k	l	m	n	o	p	q	r	s	t
> 17.567	2.123	10.20	125	40	TEST	This is a test	binary data
> *** Select data from slave to compare ***
> SELECT * FROM t1 ORDER BY a;
> a	b	c	d	e	f	g	h	j	k	l	m	n	o	p	q	r	s	t
> 17.567	2.123000000	10.200000000000000000000000000000	125	40
> *** Cleanup  ***
> DROP TABLE t1;
>
> --- New file ---
> +++ mysql-test/t/rpl_colSize.test	07/07/23 17:19:14
> ##################################################################
> # rpl_colSize                                                    #
> #                                                                #
> # This test is designed to test the changes included in WL#3228. #
> # The changes include the ability to replicate with the master   #
> # having columns that are smaller (shorter) than the slave.      #
> ##################################################################
>
> -- source include/have_binlog_format_row.inc
> -- source include/master-slave.inc
>
> --disable_warnings
> DROP TABLE IF EXISTS t1;
> --enable_warnings
>
> --echo **** Testing WL#3228 changes. ****
> --echo *** Create "wider" table on slave ***
> sync_slave_with_master;
> STOP SLAVE;
> RESET SLAVE;
>
> eval CREATE TABLE t1 (
>   a float     (47),
>   b double    (143,9),
>   c decimal   (65,30),
>   d numeric   (4,0),
>   e bit       (32),
>   f char      (21),
>   g varchar   (1300),
>   h binary    (33),
>   j varbinary (200),
>   k enum      ('5','6','7', '8','9','0'),
>   l set      
> ('1','2','3','4','5','6','7','8','9','0','11','12','13','14','15','16','17','18','19','21','22','23','24','25','26','27','28','29'),
>   m TINYBLOB,
>   n BLOB,
>   o MEDIUMBLOB,
>   p LONGBLOB,
>   q TINYTEXT,
>   r TEXT,
>   s MEDIUMTEXT,
>   t LONGTEXT
> );
>
> --echo *** Create same table on master but with narrow columns ***
> connection master;
> eval CREATE TABLE t1 (
>   a float     (44),
>   b double    (10,3),
>   c decimal   (10,2),
>   d numeric   (3,0),
>   e bit       (16),
>   f char      (10),
>   g varchar   (100),
>   h binary    (20),
>   j varbinary (20),
>   k enum      ('5','6','7'),
>   l set       ('1','2','3','4','5','6','7','8','9','0'),
>   m TINYBLOB,
>   n BLOB,
>   o MEDIUMBLOB,
>   p LONGBLOB,
>   q TINYTEXT,
>   r TEXT,
>   s MEDIUMTEXT,
>   t LONGTEXT
> );
>
> RESET MASTER;
>
> --echo *** Start replication ***
> connection slave;
> START SLAVE;
>
> --echo *** Insert data on master and display it. ***
> connection master;
>
> INSERT INTO t1 () VALUES (
>   17.567, 
>   2.123, 
>   10.20, 
>   125,
>   hex(64),
>   'TEST',
>   'This is a test',
>   'binary data',
>   'more binary data',
>   '6',
>   '7',
>   "blob 1",
>   "blob  2",
>   "blob   3",
>   "blob    4",
>   "text 1",
>   "text  2",
>   "text   3",
>   "text    4");
>
> SELECT * FROM t1 ORDER BY a;
>
> --echo *** Select data from slave to compare ***
> sync_slave_with_master;
> connection slave;
>
> SELECT * FROM t1 ORDER BY a;
>
> --echo *** Cleanup  ***
> connection master;
> DROP TABLE t1;
> sync_slave_with_master;
>
> # END 5.1 Test Case
>   

For the CHAR and VARCHAR, you need to test all combinations, i.e., for 
CHAR/VARCHAR column on master of type CHAR(m) and column on slave of 
type CHAR(s)

Master Slave
<256   <256   with m < s, m > s, and m == s
 >255   <256   with m < s, m > s, and m == s
<256   >255   with m < s, m > s, and m == s
 >255   >255   with m < s, m > s, and m == s

You also need to test BIT(m) (on master) and BIT(s) (on slave) so that:

m < s, m % 8 != 0, and s % 8 == 0
m < s, m % 8 == 0, and s % 8 != 0
m < s, m % 8 != 0, and s % 8 != 0
m > s, m % 8 != 0, and s % 8 == 0
m > s, m % 8 == 0, and s % 8 != 0
m > s, m % 8 != 0, and s % 8 != 0

There is no need to bother about bit fields that where both are 
divisible by 8, since they don't store any bits among the null bits of 
the record.

>
> --- 1.25/mysql-test/r/rpl_ndb_log.result	2007-07-23 17:19:38 -04:00
> +++ 1.26/mysql-test/r/rpl_ndb_log.result	2007-07-23 17:19:38 -04:00
> @@ -88,12 +88,12 @@
>  master-bin.000002	#	Query	1	#	COMMIT
>  show binary logs;
>  Log_name	File_size
> -master-bin.000001	1775
> -master-bin.000002	617
> +master-bin.000001	1789
> +master-bin.000002	623
>  start slave;
>  show binary logs;
>  Log_name	File_size
> -slave-bin.000001	1870
> +slave-bin.000001	1884
>  slave-bin.000002	202
>  show binlog events in 'slave-bin.000001' from 4;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> @@ -128,7 +128,7 @@
>  slave-bin.000002	#	Query	2	#	COMMIT
>  show slave status;
> 
> Slave_IO_State	Master_Host	Master_User	Master_Port	Connect_Retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_Do_DB	Replicate_Ignore_DB	Replicate_Do_Table	Replicate_Ignore_Table	Replicate_Wild_Do_Table	Replicate_Wild_Ignore_Table	Last_Errno	Last_Error	Skip_Counter	Exec_Master_Log_Pos	Relay_Log_Space	Until_Condition	Until_Log_File	Until_Log_Pos	Master_SSL_Allowed	Master_SSL_CA_File	Master_SSL_CA_Path	Master_SSL_Cert	Master_SSL_Cipher	Master_SSL_Key	Seconds_Behind_Master	Master_SSL_Verify_Server_Cert
>
> -#	127.0.0.1	root	MASTER_PORT	1	master-bin.000002	617	#	#	master-bin.000002	Yes	Yes				#			0		0	617	#	None		0	No						#	No
>
> +#	127.0.0.1	root	MASTER_PORT	1	master-bin.000002	623	#	#	master-bin.000002	Yes	Yes				#			0		0	623	#	None		0	No						#	No
>  show binlog events in 'slave-bin.000005' from 4;
>  ERROR HY000: Error when executing command SHOW BINLOG EVENTS: Could not find target
> log
>  DROP TABLE t1;
>
> --- 1.17/mysql-test/r/rpl_row_log_innodb.result	2007-07-23 17:19:38 -04:00
> +++ 1.18/mysql-test/r/rpl_row_log_innodb.result	2007-07-23 17:19:38 -04:00
> @@ -71,13 +71,13 @@
>  master-bin.000002	#	Xid	1	#	COMMIT /* XID */
>  show binary logs;
>  Log_name	File_size
> -master-bin.000001	1314
> -master-bin.000002	404
> +master-bin.000001	1320
> +master-bin.000002	406
>  start slave;
>  show binary logs;
>  Log_name	File_size
> -slave-bin.000001	1412
> -slave-bin.000002	305
> +slave-bin.000001	1418
> +slave-bin.000002	307
>  show binlog events in 'slave-bin.000001' from 4;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  slave-bin.000001	#	Format_desc	2	#	Server ver: VERSION, Binlog ver: 4
> @@ -101,7 +101,7 @@
>  slave-bin.000002	#	Xid	1	#	COMMIT /* XID */
>  show slave status;
> 
> Slave_IO_State	Master_Host	Master_User	Master_Port	Connect_Retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_Do_DB	Replicate_Ignore_DB	Replicate_Do_Table	Replicate_Ignore_Table	Replicate_Wild_Do_Table	Replicate_Wild_Ignore_Table	Last_Errno	Last_Error	Skip_Counter	Exec_Master_Log_Pos	Relay_Log_Space	Until_Condition	Until_Log_File	Until_Log_Pos	Master_SSL_Allowed	Master_SSL_CA_File	Master_SSL_CA_Path	Master_SSL_Cert	Master_SSL_Cipher	Master_SSL_Key	Seconds_Behind_Master	Master_SSL_Verify_Server_Cert
>
> -#	127.0.0.1	root	MASTER_PORT	1	master-bin.000002	404	#	#	master-bin.000002	Yes	Yes				#			0		0	404	#	None		0	No						#	No
>
> +#	127.0.0.1	root	MASTER_PORT	1	master-bin.000002	406	#	#	master-bin.000002	Yes	Yes				#			0		0	406	#	None		0	No						#	No
>  show binlog events in 'slave-bin.000005' from 4;
>  ERROR HY000: Error when executing command SHOW BINLOG EVENTS: Could not find target
> log
>  DROP TABLE t1;
>
> --- 1.12/mysql-test/r/rpl_row_create_table.result	2007-07-23 17:19:39 -04:00
> +++ 1.13/mysql-test/r/rpl_row_create_table.result	2007-07-23 17:19:39 -04:00
> @@ -127,8 +127,9 @@
>  NULL	6	12
>  CREATE TABLE t7 (UNIQUE(b)) SELECT a,b FROM tt3;
>  ERROR 23000: Duplicate entry '2' for key 'b'
> -SHOW BINLOG EVENTS FROM 1098;
> +SHOW BINLOG EVENTS FROM 1017;
>   

Start from 1100 instead of 1017 here.

>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> +#	1017	Write_rows	#	293	table_id: # flags: STMT_END_F
>  CREATE TABLE t7 (a INT, b INT UNIQUE);
>  INSERT INTO t7 SELECT a,b FROM tt3;
>  ERROR 23000: Duplicate entry '2' for key 'b'
> @@ -137,11 +138,12 @@
>  1	2
>  2	4
>  3	6
> -SHOW BINLOG EVENTS FROM 1098;
> +SHOW BINLOG EVENTS FROM 1017;
>   

Start from 1100 instead of 1017 here.

>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> -#	1098	Query	#	1198	use `test`; CREATE TABLE t7 (a INT, b INT UNIQUE)
> -#	1198	Table_map	#	1238	table_id: # (test.t7)
> -#	1238	Write_rows	#	1294	table_id: # flags: STMT_END_F
> +#	1017	Write_rows	#	293	table_id: # flags: STMT_END_F
>   
> +#	1100	Query	#	1200	use `test`; CREATE TABLE t7 (a INT, b INT UNIQUE)
> +#	1200	Table_map	#	1242	table_id: # (test.t7)
> +#	1242	Write_rows	#	1298	table_id: # flags: STMT_END_F
>  SELECT * FROM t7 ORDER BY a,b;
>  a	b
>  1	2
> @@ -154,10 +156,11 @@
>  ROLLBACK;
>  Warnings:
>  Warning	1196	Some non-transactional changed tables couldn't be rolled back
> -SHOW BINLOG EVENTS FROM 1294;
> +SHOW BINLOG EVENTS FROM 1242;
>   

Start from 1298 instead of 1242 here.

>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> -#	1294	Table_map	#	1334	table_id: # (test.t7)
> -#	1334	Write_rows	#	1390	table_id: # flags: STMT_END_F
> +#	1242	Write_rows	#	1298	table_id: # flags: STMT_END_F
> +#	1298	Table_map	#	1340	table_id: # (test.t7)
> +#	1340	Write_rows	#	1396	table_id: # flags: STMT_END_F
>  SELECT * FROM t7 ORDER BY a,b;
>  a	b
>  1	2
> @@ -192,10 +195,10 @@
>    `a` int(11) DEFAULT NULL,
>    `b` int(11) DEFAULT NULL
>  ) ENGINE=MyISAM DEFAULT CHARSET=latin1
> -SHOW BINLOG EVENTS FROM 1390;
> +SHOW BINLOG EVENTS FROM 1396;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> -#	1390	Query	#	1476	use `test`; CREATE TABLE t8 LIKE t4
> -#	1476	Query	#	1615	use `test`; CREATE TABLE `t9` (
> +#	1396	Query	#	1482	use `test`; CREATE TABLE t8 LIKE t4
> +#	1482	Query	#	1621	use `test`; CREATE TABLE `t9` (
>    `a` int(11) DEFAULT NULL,
>    `b` int(11) DEFAULT NULL
>  )
> @@ -276,31 +279,31 @@
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  #	4	Format_desc	#	106	Server ver: #, Binlog ver: #
>  #	106	Query	#	192	use `test`; CREATE TABLE t1 (a INT)
> -#	192	Table_map	#	231	table_id: # (test.t1)
> -#	231	Write_rows	#	275	table_id: # flags: STMT_END_F
> -#	275	Query	#	343	use `test`; BEGIN
> -#	343	Query	#	125	use `test`; CREATE TABLE `t2` (
> +#	192	Table_map	#	233	table_id: # (test.t1)
> +#	233	Write_rows	#	277	table_id: # flags: STMT_END_F
> +#	277	Query	#	345	use `test`; BEGIN
> +#	345	Query	#	125	use `test`; CREATE TABLE `t2` (
>    `a` int(11) DEFAULT NULL
>  ) ENGINE=InnoDB
> -#	468	Table_map	#	164	table_id: # (test.t2)
> -#	507	Write_rows	#	208	table_id: # flags: STMT_END_F
> -#	551	Xid	#	578	COMMIT /* XID */
> -#	578	Query	#	646	use `test`; BEGIN
> -#	646	Query	#	125	use `test`; CREATE TABLE `t3` (
> +#	470	Table_map	#	166	table_id: # (test.t2)
> +#	511	Write_rows	#	210	table_id: # flags: STMT_END_F
> +#	555	Xid	#	582	COMMIT /* XID */
> +#	582	Query	#	650	use `test`; BEGIN
> +#	650	Query	#	125	use `test`; CREATE TABLE `t3` (
>    `a` int(11) DEFAULT NULL
>  ) ENGINE=InnoDB
> -#	771	Table_map	#	164	table_id: # (test.t3)
> -#	810	Write_rows	#	208	table_id: # flags: STMT_END_F
> -#	854	Xid	#	881	COMMIT /* XID */
> -#	881	Query	#	949	use `test`; BEGIN
> -#	949	Query	#	125	use `test`; CREATE TABLE `t4` (
> +#	775	Table_map	#	166	table_id: # (test.t3)
> +#	816	Write_rows	#	210	table_id: # flags: STMT_END_F
> +#	860	Xid	#	887	COMMIT /* XID */
> +#	887	Query	#	955	use `test`; BEGIN
> +#	955	Query	#	125	use `test`; CREATE TABLE `t4` (
>    `a` int(11) DEFAULT NULL
>  ) ENGINE=InnoDB
> -#	1074	Table_map	#	164	table_id: # (test.t4)
> -#	1113	Write_rows	#	208	table_id: # flags: STMT_END_F
> -#	1157	Xid	#	1184	COMMIT /* XID */
> -#	1184	Table_map	#	1223	table_id: # (test.t1)
> -#	1223	Write_rows	#	1267	table_id: # flags: STMT_END_F
> +#	1080	Table_map	#	166	table_id: # (test.t4)
> +#	1121	Write_rows	#	210	table_id: # flags: STMT_END_F
> +#	1165	Xid	#	1192	COMMIT /* XID */
> +#	1192	Table_map	#	1233	table_id: # (test.t1)
> +#	1233	Write_rows	#	1277	table_id: # flags: STMT_END_F
>  SHOW TABLES;
>  Tables_in_test
>  t1
> @@ -367,15 +370,15 @@
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  #	4	Format_desc	#	106	Server ver: #, Binlog ver: #
>  #	106	Query	#	192	use `test`; CREATE TABLE t1 (a INT)
> -#	192	Table_map	#	231	table_id: # (test.t1)
> -#	231	Write_rows	#	275	table_id: # flags: STMT_END_F
> -#	275	Query	#	375	use `test`; CREATE TABLE t2 (a INT) ENGINE=INNODB
> -#	375	Query	#	443	use `test`; BEGIN
> -#	443	Table_map	#	39	table_id: # (test.t2)
> -#	482	Write_rows	#	83	table_id: # flags: STMT_END_F
> -#	526	Table_map	#	122	table_id: # (test.t2)
> -#	565	Write_rows	#	161	table_id: # flags: STMT_END_F
> -#	604	Xid	#	631	COMMIT /* XID */
> +#	192	Table_map	#	233	table_id: # (test.t1)
> +#	233	Write_rows	#	277	table_id: # flags: STMT_END_F
> +#	277	Query	#	377	use `test`; CREATE TABLE t2 (a INT) ENGINE=INNODB
> +#	377	Query	#	445	use `test`; BEGIN
> +#	445	Table_map	#	41	table_id: # (test.t2)
> +#	486	Write_rows	#	85	table_id: # flags: STMT_END_F
> +#	530	Table_map	#	126	table_id: # (test.t2)
> +#	571	Write_rows	#	165	table_id: # flags: STMT_END_F
> +#	610	Xid	#	637	COMMIT /* XID */
>  SELECT * FROM t2 ORDER BY a;
>  a
>  1
> @@ -394,10 +397,10 @@
>  ROLLBACK;
>  SELECT * FROM t2 ORDER BY a;
>  a
> -SHOW BINLOG EVENTS FROM 631;
> +SHOW BINLOG EVENTS FROM 637;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> -#	631	Query	#	80	use `test`; TRUNCATE TABLE t2
> -#	711	Xid	#	738	COMMIT /* XID */
> +#	637	Query	#	80	use `test`; TRUNCATE TABLE t2
> +#	717	Xid	#	744	COMMIT /* XID */
>  SELECT * FROM t2 ORDER BY a;
>  a
>  DROP TABLE t1,t2;
>
> --- 1.5/mysql-test/r/rpl_row_flsh_tbls.result	2007-07-23 17:19:39 -04:00
> +++ 1.6/mysql-test/r/rpl_row_flsh_tbls.result	2007-07-23 17:19:39 -04:00
> @@ -12,13 +12,13 @@
>  insert into t4 select * from t3;
>  rename table t1 to t5, t2 to t1;
>  flush no_write_to_binlog tables;
> -SHOW BINLOG EVENTS FROM 619 ;
> +SHOW BINLOG EVENTS FROM 623 ;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  master-bin.000001	#	Query	1	#	use `test`; rename table t1 to t5, t2 to t1
>  select * from t3;
>  a
>  flush tables;
> -SHOW BINLOG EVENTS FROM 619 ;
> +SHOW BINLOG EVENTS FROM 623 ;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  master-bin.000001	#	Query	1	#	use `test`; rename table t1 to t5, t2 to t1
>  master-bin.000001	#	Query	1	#	use `test`; flush tables
>
> --- 1.7/mysql-test/r/rpl_row_inexist_tbl.result	2007-07-23 17:19:39 -04:00
> +++ 1.8/mysql-test/r/rpl_row_inexist_tbl.result	2007-07-23 17:19:39 -04:00
> @@ -39,7 +39,7 @@
>  Last_Errno	1146
>  Last_Error	Error 'Table 'test.t1' doesn't exist' on opening table `test`.`t1`
>  Skip_Counter	0
> -Exec_Master_Log_Pos	524
> +Exec_Master_Log_Pos	530
>  Relay_Log_Space	#
>  Until_Condition	None
>  Until_Log_File	
>
> --- 1.17/mysql-test/r/rpl_row_log.result	2007-07-23 17:19:39 -04:00
> +++ 1.18/mysql-test/r/rpl_row_log.result	2007-07-23 17:19:39 -04:00
> @@ -66,13 +66,13 @@
>  master-bin.000002	#	Write_rows	1	#	table_id: # flags: STMT_END_F
>  show binary logs;
>  Log_name	File_size
> -master-bin.000001	1260
> -master-bin.000002	377
> +master-bin.000001	1266
> +master-bin.000002	379
>  start slave;
>  show binary logs;
>  Log_name	File_size
> -slave-bin.000001	1358
> -slave-bin.000002	278
> +slave-bin.000001	1364
> +slave-bin.000002	280
>  show binlog events in 'slave-bin.000001' from 4;
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  slave-bin.000001	#	Format_desc	2	#	Server ver: VERSION, Binlog ver: 4
> @@ -93,7 +93,7 @@
>  slave-bin.000002	#	Write_rows	1	#	table_id: # flags: STMT_END_F
>  show slave status;
> 
> Slave_IO_State	Master_Host	Master_User	Master_Port	Connect_Retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_Do_DB	Replicate_Ignore_DB	Replicate_Do_Table	Replicate_Ignore_Table	Replicate_Wild_Do_Table	Replicate_Wild_Ignore_Table	Last_Errno	Last_Error	Skip_Counter	Exec_Master_Log_Pos	Relay_Log_Space	Until_Condition	Until_Log_File	Until_Log_Pos	Master_SSL_Allowed	Master_SSL_CA_File	Master_SSL_CA_Path	Master_SSL_Cert	Master_SSL_Cipher	Master_SSL_Key	Seconds_Behind_Master	Master_SSL_Verify_Server_Cert
>
> -#	127.0.0.1	root	MASTER_PORT	1	master-bin.000002	377	#	#	master-bin.000002	Yes	Yes				#			0		0	377	#	None		0	No						#	No
>
> +#	127.0.0.1	root	MASTER_PORT	1	master-bin.000002	379	#	#	master-bin.000002	Yes	Yes				#			0		0	379	#	None		0	No						#	No
>  show binlog events in 'slave-bin.000005' from 4;
>  ERROR HY000: Error when executing command SHOW BINLOG EVENTS: Could not find target
> log
>  DROP TABLE t1;
>
> --- 1.9/mysql-test/r/rpl_row_max_relay_size.result	2007-07-23 17:19:39 -04:00
> +++ 1.10/mysql-test/r/rpl_row_max_relay_size.result	2007-07-23 17:19:39 -04:00
> @@ -30,7 +30,7 @@
>  Master_Port	MASTER_PORT
>  Connect_Retry	1
>  Master_Log_File	master-bin.000001
> -Read_Master_Log_Pos	58668
> +Read_Master_Log_Pos	60268
>  Relay_Log_File	#
>  Relay_Log_Pos	#
>  Relay_Master_Log_File	master-bin.000001
> @@ -45,7 +45,7 @@
>  Last_Errno	0
>  Last_Error	
>  Skip_Counter	0
> -Exec_Master_Log_Pos	58668
> +Exec_Master_Log_Pos	60268
>  Relay_Log_Space	#
>  Until_Condition	None
>  Until_Log_File	
> @@ -74,7 +74,7 @@
>  Master_Port	MASTER_PORT
>  Connect_Retry	1
>  Master_Log_File	master-bin.000001
> -Read_Master_Log_Pos	58668
> +Read_Master_Log_Pos	60268
>  Relay_Log_File	#
>  Relay_Log_Pos	#
>  Relay_Master_Log_File	master-bin.000001
> @@ -89,7 +89,7 @@
>  Last_Errno	0
>  Last_Error	
>  Skip_Counter	0
> -Exec_Master_Log_Pos	58668
> +Exec_Master_Log_Pos	60268
>  Relay_Log_Space	#
>  Until_Condition	None
>  Until_Log_File	
> @@ -118,7 +118,7 @@
>  Master_Port	MASTER_PORT
>  Connect_Retry	1
>  Master_Log_File	master-bin.000001
> -Read_Master_Log_Pos	58668
> +Read_Master_Log_Pos	60268
>  Relay_Log_File	#
>  Relay_Log_Pos	#
>  Relay_Master_Log_File	master-bin.000001
> @@ -133,7 +133,7 @@
>  Last_Errno	0
>  Last_Error	
>  Skip_Counter	0
> -Exec_Master_Log_Pos	58668
> +Exec_Master_Log_Pos	60268
>  Relay_Log_Space	#
>  Until_Condition	None
>  Until_Log_File	
> @@ -201,7 +201,7 @@
>  Master_Port	MASTER_PORT
>  Connect_Retry	1
>  Master_Log_File	master-bin.000001
> -Read_Master_Log_Pos	58754
> +Read_Master_Log_Pos	60354
>  Relay_Log_File	#
>  Relay_Log_Pos	#
>  Relay_Master_Log_File	master-bin.000001
> @@ -216,7 +216,7 @@
>  Last_Errno	0
>  Last_Error	
>  Skip_Counter	0
> -Exec_Master_Log_Pos	58754
> +Exec_Master_Log_Pos	60354
>  Relay_Log_Space	#
>  Until_Condition	None
>  Until_Log_File	
> @@ -241,7 +241,7 @@
>  Master_Port	MASTER_PORT
>  Connect_Retry	1
>  Master_Log_File	master-bin.000001
> -Read_Master_Log_Pos	58830
> +Read_Master_Log_Pos	60430
>  Relay_Log_File	#
>  Relay_Log_Pos	#
>  Relay_Master_Log_File	master-bin.000001
> @@ -256,7 +256,7 @@
>  Last_Errno	0
>  Last_Error	
>  Skip_Counter	0
> -Exec_Master_Log_Pos	58830
> +Exec_Master_Log_Pos	60430
>  Relay_Log_Space	#
>  Until_Condition	None
>  Until_Log_File	
>
> --- 1.5/mysql-test/r/rpl_row_until.result	2007-07-23 17:19:39 -04:00
> +++ 1.6/mysql-test/r/rpl_row_until.result	2007-07-23 17:19:39 -04:00
> @@ -21,7 +21,7 @@
>  4
>  show slave status;
> 
> Slave_IO_State	Master_Host	Master_User	Master_Port	Connect_Retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_Do_DB	Replicate_Ignore_DB	Replicate_Do_Table	Replicate_Ignore_Table	Replicate_Wild_Do_Table	Replicate_Wild_Ignore_Table	Last_Errno	Last_Error	Skip_Counter	Exec_Master_Log_Pos	Relay_Log_Space	Until_Condition	Until_Log_File	Until_Log_Pos	Master_SSL_Allowed	Master_SSL_CA_File	Master_SSL_CA_Path	Master_SSL_Cert	Master_SSL_Cipher	Master_SSL_Key	Seconds_Behind_Master	Master_SSL_Verify_Server_Cert
>
> -#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	744	slave-relay-bin.000004	#	master-bin.000001	#	No							0		0	315	#	Master	master-bin.000001	311	No						#	No
>
> +#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	750	slave-relay-bin.000004	#	master-bin.000001	#	No							0		0	317	#	Master	master-bin.000001	311	No						#	No
>  start slave until master_log_file='master-no-such-bin.000001', master_log_pos=291;
>  select * from t1;
>  n
> @@ -31,7 +31,7 @@
>  4
>  show slave status;
> 
> Slave_IO_State	Master_Host	Master_User	Master_Port	Connect_Retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_Do_DB	Replicate_Ignore_DB	Replicate_Do_Table	Replicate_Ignore_Table	Replicate_Wild_Do_Table	Replicate_Wild_Ignore_Table	Last_Errno	Last_Error	Skip_Counter	Exec_Master_Log_Pos	Relay_Log_Space	Until_Condition	Until_Log_File	Until_Log_Pos	Master_SSL_Allowed	Master_SSL_CA_File	Master_SSL_CA_Path	Master_SSL_Cert	Master_SSL_Cipher	Master_SSL_Key	Seconds_Behind_Master	Master_SSL_Verify_Server_Cert
>
> -#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	744	slave-relay-bin.000004	#	master-bin.000001	#	No							0		0	315	#	Master	master-no-such-bin.000001	291	No						#	No
>
> +#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	750	slave-relay-bin.000004	#	master-bin.000001	#	No							0		0	317	#	Master	master-no-such-bin.000001	291	No						#	No
>  start slave until relay_log_file='slave-relay-bin.000004', relay_log_pos=728;
>  select * from t2;
>  n
> @@ -39,13 +39,13 @@
>  2
>  show slave status;
> 
> Slave_IO_State	Master_Host	Master_User	Master_Port	Connect_Retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_Do_DB	Replicate_Ignore_DB	Replicate_Do_Table	Replicate_Ignore_Table	Replicate_Wild_Do_Table	Replicate_Wild_Ignore_Table	Last_Errno	Last_Error	Skip_Counter	Exec_Master_Log_Pos	Relay_Log_Space	Until_Condition	Until_Log_File	Until_Log_Pos	Master_SSL_Allowed	Master_SSL_CA_File	Master_SSL_CA_Path	Master_SSL_Cert	Master_SSL_Cipher	Master_SSL_Key	Seconds_Behind_Master	Master_SSL_Verify_Server_Cert
>
> -#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	744	slave-relay-bin.000004	#	master-bin.000001	#	No							0		0	590	#	Relay	slave-relay-bin.000004	728	No						#	No
>
> +#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	750	slave-relay-bin.000004	#	master-bin.000001	#	No							0		0	594	#	Relay	slave-relay-bin.000004	728	No						#	No
>  start slave;
>  stop slave;
>  start slave until master_log_file='master-bin.000001', master_log_pos=740;
>  show slave status;
> 
> Slave_IO_State	Master_Host	Master_User	Master_Port	Connect_Retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_Do_DB	Replicate_Ignore_DB	Replicate_Do_Table	Replicate_Ignore_Table	Replicate_Wild_Do_Table	Replicate_Wild_Ignore_Table	Last_Errno	Last_Error	Skip_Counter	Exec_Master_Log_Pos	Relay_Log_Space	Until_Condition	Until_Log_File	Until_Log_Pos	Master_SSL_Allowed	Master_SSL_CA_File	Master_SSL_CA_Path	Master_SSL_Cert	Master_SSL_Cipher	Master_SSL_Key	Seconds_Behind_Master	Master_SSL_Verify_Server_Cert
>
> -#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	744	slave-relay-bin.000004	#	master-bin.000001	Yes	No							0		0	744	#	Master	master-bin.000001	740	No						#	No
>
> +#	127.0.0.1	root	MASTER_MYPORT	1	master-bin.000001	750	slave-relay-bin.000004	#	master-bin.000001	Yes	No							0		0	750	#	Master	master-bin.000001	740	No						#	No
>  start slave until master_log_file='master-bin', master_log_pos=561;
>  ERROR HY000: Incorrect parameter or combination of parameters for START SLAVE UNTIL
>  start slave until master_log_file='master-bin.000001', master_log_pos=561,
> relay_log_pos=12;
>
> --- 1.9/mysql-test/t/binlog_row_mix_innodb_myisam.test	2007-07-23 17:19:39 -04:00
> +++ 1.10/mysql-test/t/binlog_row_mix_innodb_myisam.test	2007-07-23 17:19:39 -04:00
> @@ -20,7 +20,7 @@
>  # ER_SERVER_SHUTDOWN (i.e. disconnection just rolls back transaction
>  # and does not make slave to stop)
>  flush logs;
> ---exec $MYSQL_BINLOG --start-position=520 $MYSQLTEST_VARDIR/log/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/mix_innodb_myisam_binlog.output
> +--exec $MYSQL_BINLOG --start-position=524 $MYSQLTEST_VARDIR/log/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/mix_innodb_myisam_binlog.output
>  --replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
>  eval select
>  (@a:=load_file("$MYSQLTEST_VARDIR/tmp/mix_innodb_myisam_binlog.output"))
>
> --- 1.12/mysql-test/t/rpl_row_create_table.test	2007-07-23 17:19:39 -04:00
> +++ 1.13/mysql-test/t/rpl_row_create_table.test	2007-07-23 17:19:39 -04:00
> @@ -72,7 +72,7 @@
>  # Shouldn't be written to the binary log
>  --replace_column 1 # 4 #
>  --replace_regex /\/\* xid=.* \*\//\/* XID *\// /table_id: [0-9]+/table_id: #/
> -SHOW BINLOG EVENTS FROM 1098;
> +SHOW BINLOG EVENTS FROM 1017;
>  
>  # Test that INSERT-SELECT works the same way as for SBR.
>  CREATE TABLE t7 (a INT, b INT UNIQUE);
> @@ -82,7 +82,7 @@
>  # Should be written to the binary log
>  --replace_column 1 # 4 #
>  --replace_regex /\/\* xid=.* \*\//\/* XID *\// /table_id: [0-9]+/table_id: #/
> -SHOW BINLOG EVENTS FROM 1098;
> +SHOW BINLOG EVENTS FROM 1017;
>  sync_slave_with_master;
>  SELECT * FROM t7 ORDER BY a,b;
>  
> @@ -94,7 +94,7 @@
>  ROLLBACK;
>  --replace_column 1 # 4 #
>  --replace_regex /\/\* xid=.* \*\//\/* XID *\// /table_id: [0-9]+/table_id: #/
> -SHOW BINLOG EVENTS FROM 1294;
> +SHOW BINLOG EVENTS FROM 1242;
>  SELECT * FROM t7 ORDER BY a,b;
>  sync_slave_with_master;
>  SELECT * FROM t7 ORDER BY a,b;
> @@ -110,7 +110,7 @@
>  --query_vertical SHOW CREATE TABLE t9
>  --replace_column 1 # 4 #
>  --replace_regex /\/\* xid=.* \*\//\/* XID *\// /table_id: [0-9]+/table_id: #/
> -SHOW BINLOG EVENTS FROM 1390;
> +SHOW BINLOG EVENTS FROM 1396;
>  sync_slave_with_master;
>  --echo **** On Slave ****
>  --query_vertical SHOW CREATE TABLE t8
> @@ -227,7 +227,7 @@
>  SELECT * FROM t2 ORDER BY a;
>  --replace_column 1 # 4 #
>  --replace_regex /\/\* xid=.* \*\//\/* XID *\// /Server ver: .*, Binlog ver:
> .*/Server ver: #, Binlog ver: #/ /table_id: [0-9]+/table_id: #/
> -SHOW BINLOG EVENTS FROM 631;
> +SHOW BINLOG EVENTS FROM 637;
>  sync_slave_with_master;
>  SELECT * FROM t2 ORDER BY a;
>  
>
> --- 1.5/mysql-test/t/rpl_row_flsh_tbls.test	2007-07-23 17:19:39 -04:00
> +++ 1.6/mysql-test/t/rpl_row_flsh_tbls.test	2007-07-23 17:19:39 -04:00
> @@ -1,7 +1,7 @@
>  # depends on the binlog output
>  -- source include/have_binlog_format_row.inc
>  
> -let $rename_event_pos= 619;
> +let $rename_event_pos= 623;
>  
>  # Bug#18326: Do not lock table for writing during prepare of statement
>  # The use of the ps protocol causes extra table maps in the binlog, so
>
> --- 1.14/sql/rpl_rli.h	2007-07-23 17:19:39 -04:00
> +++ 1.15/sql/rpl_rli.h	2007-07-23 17:19:39 -04:00
> @@ -19,6 +19,7 @@
>  #define MAX_SLAVE_ERRMSG      1024
>  
>  #include "rpl_tblmap.h"
> +#include "rpl_utility.h"
>  
>  struct RPL_TABLE_LIST;
>  
> @@ -304,6 +305,15 @@
>    RPL_TABLE_LIST *tables_to_lock;           /* RBR: Tables to lock  */
>    uint tables_to_lock_count;        /* RBR: Count of tables to lock */
>    table_mapping m_table_map;      /* RBR: Mapping table-id to table */
> +
> +  inline table_def *get_tabledef(TABLE *tbl)
> +  {
> +    table_def *td= 0;
> +    for (TABLE_LIST *ptr= tables_to_lock; ptr && !td; ptr=
> ptr->next_global)
> +      if (ptr->table == tbl)
> +        td= &((RPL_TABLE_LIST *)ptr)->m_tabledef;
> +    return (td);
> +  }
>  
>    /*
>      Last charset (6 bytes) seen by slave SQL thread is cached here; it helps
>
> --- 1.16/mysql-test/r/rpl_row_basic_11bugs.result	2007-07-23 17:19:39 -04:00
> +++ 1.17/mysql-test/r/rpl_row_basic_11bugs.result	2007-07-23 17:19:39 -04:00
> @@ -56,8 +56,8 @@
>  Log_name	Pos	Event_type	Server_id	End_log_pos	Info
>  master-bin.000001	4	Format_desc	1	106	Server ver: SERVER_VERSION, Binlog ver: 4
>  master-bin.000001	106	Query	1	192	use `test`; CREATE TABLE t1 (a INT)
> -master-bin.000001	192	Table_map	1	231	table_id: # (test.t1)
> -master-bin.000001	231	Write_rows	1	270	table_id: # flags: STMT_END_F
> +master-bin.000001	192	Table_map	1	233	table_id: # (test.t1)
> +master-bin.000001	233	Write_rows	1	272	table_id: # flags: STMT_END_F
>  DROP TABLE t1;
>  ================ Test for BUG#17620 ================
>  drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>
> --- 1.10/mysql-test/r/rpl_truncate_7ndb.result	2007-07-23 17:19:39 -04:00
> +++ 1.11/mysql-test/r/rpl_truncate_7ndb.result	2007-07-23 17:19:39 -04:00
> @@ -32,14 +32,14 @@
>  master-bin.000001	4	Format_desc	1	106	Server ver: SERVER_VERSION, Binlog ver: 4
>  master-bin.000001	106	Query	1	223	use `test`; CREATE TABLE t1 (a INT PRIMARY KEY, b
> LONG) ENGINE=NDB
>  master-bin.000001	223	Query	1	287	BEGIN
> -master-bin.000001	287	Table_map	1	40	table_id: # (test.t1)
> -master-bin.000001	327	Table_map	1	98	table_id: # (mysql.ndb_apply_status)
> -master-bin.000001	385	Write_rows	1	157	table_id: #
> -master-bin.000001	444	Write_rows	1	195	table_id: #
> -master-bin.000001	482	Write_rows	1	233	table_id: # flags: STMT_END_F
> -master-bin.000001	520	Query	1	585	COMMIT
> -master-bin.000001	585	Query	1	665	use `test`; TRUNCATE TABLE t1
> -master-bin.000001	665	Query	1	741	use `test`; DROP TABLE t1
> +master-bin.000001	287	Table_map	1	43	table_id: # (test.t1)
> +master-bin.000001	330	Table_map	1	105	table_id: # (mysql.ndb_apply_status)
> +master-bin.000001	392	Write_rows	1	164	table_id: #
> +master-bin.000001	451	Write_rows	1	202	table_id: #
> +master-bin.000001	489	Write_rows	1	240	table_id: # flags: STMT_END_F
> +master-bin.000001	527	Query	1	592	COMMIT
> +master-bin.000001	592	Query	1	672	use `test`; TRUNCATE TABLE t1
> +master-bin.000001	672	Query	1	748	use `test`; DROP TABLE t1
>  **** On Master ****
>  CREATE TABLE t1 (a INT PRIMARY KEY, b LONG) ENGINE=NDB;
>  INSERT INTO t1 VALUES (1,1), (2,2);
> @@ -69,27 +69,27 @@
>  master-bin.000001	4	Format_desc	1	106	Server ver: SERVER_VERSION, Binlog ver: 4
>  master-bin.000001	106	Query	1	223	use `test`; CREATE TABLE t1 (a INT PRIMARY KEY, b
> LONG) ENGINE=NDB
>  master-bin.000001	223	Query	1	287	BEGIN
> -master-bin.000001	287	Table_map	1	40	table_id: # (test.t1)
> -master-bin.000001	327	Table_map	1	98	table_id: # (mysql.ndb_apply_status)
> -master-bin.000001	385	Write_rows	1	157	table_id: #
> -master-bin.000001	444	Write_rows	1	195	table_id: #
> -master-bin.000001	482	Write_rows	1	233	table_id: # flags: STMT_END_F
> -master-bin.000001	520	Query	1	585	COMMIT
> -master-bin.000001	585	Query	1	665	use `test`; TRUNCATE TABLE t1
> -master-bin.000001	665	Query	1	741	use `test`; DROP TABLE t1
> -master-bin.000001	741	Query	1	858	use `test`; CREATE TABLE t1 (a INT PRIMARY KEY, b
> LONG) ENGINE=NDB
> -master-bin.000001	858	Query	1	922	BEGIN
> -master-bin.000001	922	Table_map	1	40	table_id: # (test.t1)
> -master-bin.000001	962	Table_map	1	98	table_id: # (mysql.ndb_apply_status)
> -master-bin.000001	1020	Write_rows	1	157	table_id: #
> -master-bin.000001	1079	Write_rows	1	195	table_id: #
> -master-bin.000001	1117	Write_rows	1	233	table_id: # flags: STMT_END_F
> -master-bin.000001	1155	Query	1	1220	COMMIT
> -master-bin.000001	1220	Query	1	1284	BEGIN
> -master-bin.000001	1284	Table_map	1	40	table_id: # (test.t1)
> -master-bin.000001	1324	Table_map	1	98	table_id: # (mysql.ndb_apply_status)
> -master-bin.000001	1382	Write_rows	1	157	table_id: #
> -master-bin.000001	1441	Delete_rows	1	191	table_id: #
> -master-bin.000001	1475	Delete_rows	1	225	table_id: # flags: STMT_END_F
> -master-bin.000001	1509	Query	1	1574	COMMIT
> -master-bin.000001	1574	Query	1	1650	use `test`; DROP TABLE t1
> +master-bin.000001	287	Table_map	1	43	table_id: # (test.t1)
> +master-bin.000001	330	Table_map	1	105	table_id: # (mysql.ndb_apply_status)
> +master-bin.000001	392	Write_rows	1	164	table_id: #
> +master-bin.000001	451	Write_rows	1	202	table_id: #
> +master-bin.000001	489	Write_rows	1	240	table_id: # flags: STMT_END_F
> +master-bin.000001	527	Query	1	592	COMMIT
> +master-bin.000001	592	Query	1	672	use `test`; TRUNCATE TABLE t1
> +master-bin.000001	672	Query	1	748	use `test`; DROP TABLE t1
> +master-bin.000001	748	Query	1	865	use `test`; CREATE TABLE t1 (a INT PRIMARY KEY, b
> LONG) ENGINE=NDB
> +master-bin.000001	865	Query	1	929	BEGIN
> +master-bin.000001	929	Table_map	1	43	table_id: # (test.t1)
> +master-bin.000001	972	Table_map	1	105	table_id: # (mysql.ndb_apply_status)
> +master-bin.000001	1034	Write_rows	1	164	table_id: #
> +master-bin.000001	1093	Write_rows	1	202	table_id: #
> +master-bin.000001	1131	Write_rows	1	240	table_id: # flags: STMT_END_F
> +master-bin.000001	1169	Query	1	1234	COMMIT
> +master-bin.000001	1234	Query	1	1298	BEGIN
> +master-bin.000001	1298	Table_map	1	43	table_id: # (test.t1)
> +master-bin.000001	1341	Table_map	1	105	table_id: # (mysql.ndb_apply_status)
> +master-bin.000001	1403	Write_rows	1	164	table_id: #
> +master-bin.000001	1462	Delete_rows	1	198	table_id: #
> +master-bin.000001	1496	Delete_rows	1	232	table_id: # flags: STMT_END_F
> +master-bin.000001	1530	Query	1	1595	COMMIT
> +master-bin.000001	1595	Query	1	1671	use `test`; DROP TABLE t1
>
> --- 1.5/sql/rpl_utility.cc	2007-07-23 17:19:39 -04:00
> +++ 1.6/sql/rpl_utility.cc	2007-07-23 17:19:39 -04:00
> @@ -15,17 +15,46 @@
>  
>  #include "rpl_utility.h"
>  
> -uint32
> -field_length_from_packed(enum_field_types const field_type, 
> -                         byte const *const data)
> +
> +/*********************************************************************
> + *                   table_def member definitions                    *
> + *********************************************************************/
> +
> +/*
> +  This function returns the field size in raw bytes based on the type
> +  and the encoded field data from the master's raw data.
> +*/
> +uint32 table_def::get_field_size(uint col, uchar *master_data)
>  {
>    uint32 length;
>  
> -  switch (field_type) {
> -  case MYSQL_TYPE_DECIMAL:
> +  switch (type(col)) {
>    case MYSQL_TYPE_NEWDECIMAL:
> -    length= ~(uint32) 0;
> +    length= my_decimal_get_binary_size(m_field_metadata[col] >> 8, 
> +             m_field_metadata[col] - ((m_field_metadata[col] >> 8) <<
> 8));
> +    break;
> +  case MYSQL_TYPE_DECIMAL:
> +  case MYSQL_TYPE_FLOAT:
> +  case MYSQL_TYPE_DOUBLE:
> +    length= m_field_metadata[col];
> +    break;
> +  case MYSQL_TYPE_SET:
> +  case MYSQL_TYPE_ENUM:
> +  case MYSQL_TYPE_STRING:
> +  {
> +    if ((((m_field_metadata[col] >> 8U) & 0x00ff) == MYSQL_TYPE_SET) ||
>   
The following version shifts the constant instead (which the compiler 
can handle), which saves a few instructions.  Not critical though, so do 
as you like.

    (m_field_metadata[col] & 0xFF00) == (MYSQL_TYPE_SET << 8)

> +        (((m_field_metadata[col] >> 8U) & 0x00ff) == MYSQL_TYPE_ENUM))
> +      length= m_field_metadata[col] & 0x00ff;
> +    else
> +    {
> +      length= m_field_metadata[col] & 0x00ff;
> +      if (length > 255)
> +        length= uint2korr(master_data) + 2;
> +      else
> +        length= (uint) *master_data + 1;
> +    }
>      break;
> +  }
>    case MYSQL_TYPE_YEAR:
>    case MYSQL_TYPE_TINY:
>      length= 1;
> @@ -44,12 +73,6 @@
>      length= 8;
>      break;
>  #endif
> -  case MYSQL_TYPE_FLOAT:
> -    length= sizeof(float);
> -    break;
> -  case MYSQL_TYPE_DOUBLE:
> -    length= sizeof(double);
> -    break;
>    case MYSQL_TYPE_NULL:
>      length= 0;
>      break;
> @@ -57,8 +80,6 @@
>      length= 3;
>      break;
>    case MYSQL_TYPE_DATE:
> -    length= 4;
> -    break;
>    case MYSQL_TYPE_TIME:
>      length= 3;
>      break;
> @@ -68,41 +89,34 @@
>    case MYSQL_TYPE_DATETIME:
>      length= 8;
>      break;
> -    break;
>    case MYSQL_TYPE_BIT:
> -    length= ~(uint32) 0;
> -    break;
> -  default:
> -    /* This case should never be chosen */
> -    DBUG_ASSERT(0);
> -    /* If something goes awfully wrong, it's better to get a string than die */
> -  case MYSQL_TYPE_STRING:
> -    length= uint2korr(data);
> +  {
> +    uint from_bit_len= m_field_metadata[col] >> 8U;
> +    uint from_len= m_field_metadata[col] - (from_bit_len << 8U);
> +    length= from_len + ((from_bit_len > 0) ? 1 : 0);
>      break;
> -
> -  case MYSQL_TYPE_ENUM:
> -  case MYSQL_TYPE_SET:
> -  case MYSQL_TYPE_VAR_STRING:
> +  }
>    case MYSQL_TYPE_VARCHAR:
> -    length= ~(uint32) 0;                               // NYI
> +    length= m_field_metadata[col] > 255 ? 2 : 1; // c&p of
> Field_varstring::data_length()
> +    length+= length == 1 ? (uint32) *master_data : uint2korr(master_data);
>      break;
> -
>    case MYSQL_TYPE_TINY_BLOB:
>    case MYSQL_TYPE_MEDIUM_BLOB:
>    case MYSQL_TYPE_LONG_BLOB:
>    case MYSQL_TYPE_BLOB:
>    case MYSQL_TYPE_GEOMETRY:
> -    length= ~(uint32) 0;                               // NYI
> +  {
> +    Field_blob *fb= new Field_blob(m_field_metadata[col]);
> +    length= fb->get_packed_size(master_data);
> +    delete fb;
>   

Ugh... to allocate a new object just to get the size seems like a waste 
of cycles. To allocate a new object *dynamically* (on the heap) just to 
get the size is really a waste of cycles. Can't you get the size some 
other way?

>      break;
>    }
> -
> +  default:
> +    length= -1;
> +  }
>    return length;
>  }
>  
> -/*********************************************************************
> - *                   table_def member definitions                    *
> - *********************************************************************/
> -
>  /*
>    Is the definition compatible with a table?
>  
> @@ -135,8 +149,14 @@
>                      tsh->fields);
>    }
>  
> +  /*
> +    We now check for column type and size compatibility.
> +  */
>    for (uint col= 0 ; col < cols_to_check ; ++col)
>    {
> +    /*
> +      Checking types.
> +    */
>      if (table->field[col]->type() != type(col))
>      {
>        DBUG_ASSERT(col < size() && col < tsh->fields);
>
> --- 1.5/sql/rpl_utility.h	2007-07-23 17:19:39 -04:00
> +++ 1.6/sql/rpl_utility.h	2007-07-23 17:19:39 -04:00
> @@ -21,13 +21,11 @@
>  #endif
>  
>  #include "mysql_priv.h"
> +#include "my_bitmap.h"
>  
>  struct st_relay_log_info;
>  typedef st_relay_log_info RELAY_LOG_INFO;
>  
> -uint32
> -field_length_from_packed(enum_field_types field_type, byte const *data);
> -
>  /**
>    A table definition from the master.
>  
> @@ -57,19 +55,98 @@
>  
>      @param types Array of types
>      @param size  Number of elements in array 'types'
> +    @param field_metadata Array of extra information about fields
> +    @param metadata_size Size of the field_metadata array
> +    @param null_bitmap The bitmap of fields that can be null
>     */
> -  table_def(field_type *types, my_size_t size)
> -    : m_size(size), m_type(new unsigned char [size])
> +  table_def(field_type *types, ulong size, uchar *field_metadata, 
> +      int metadata_size, uchar *null_bitmap)
> +    : m_size(size), m_type(new unsigned char [size]),
> +      m_field_metadata(0), m_null_bits(0)
>    {
>      if (m_type)
>        memcpy(m_type, types, size);
>      else
>        m_size= 0;
> +    /*
> +      Extract the data from the table map into the field metadata array
> +      iff there is field metadata. The variable metadata_size will be
> +      0 if we are replicating from a down-level server hence no field
>   

s/down-level server hence/old version server since/

> +      metadata was written to the table map. This can also happen if 
> +      there were no fields in the master that needed extra metadata.
> +    */
> +    if (m_size && metadata_size)
> +    { 
> +      m_field_metadata= new short int[m_size];
>   

Use my_multi_malloc instead (if possible)?

> +      int index= 0;
> +      for (unsigned int i= 0; i < m_size; i++)
> +      {
> +        switch (m_type[i]) {
> +        case MYSQL_TYPE_TINY_BLOB:
> +        case MYSQL_TYPE_BLOB:
> +        case MYSQL_TYPE_MEDIUM_BLOB:
> +        case MYSQL_TYPE_LONG_BLOB:
> +        case MYSQL_TYPE_DOUBLE:
> +        case MYSQL_TYPE_FLOAT:
> +        {
> +          /*
> +            These types store a single byte.
> +          */
> +          m_field_metadata[i]= (uchar)field_metadata[index];
>   

Is the cast necessary?

> +          index++;
> +          break;
> +        }
> +        case MYSQL_TYPE_BIT:
> +        case MYSQL_TYPE_SET:
> +        case MYSQL_TYPE_ENUM:
> +        case MYSQL_TYPE_STRING:
> +        {
> +          short int x= field_metadata[index] << 8U; // real_type
> +          index++;
> +          x = x + field_metadata[index];            // pack or field length
> +          m_field_metadata[i]= x;
> +          index++;
>   

Stepping the index once two steps (last in the code block) instead of 
twice one step (inside the code block) will be more efficient and avoids 
confusion since it is obvious where the bytes are read.

> +          break;
> +        }
> +        case MYSQL_TYPE_VARCHAR:
> +        {
> +          /*
> +            These types store two bytes.
> +          */
> +          short int *x= (short int *)&field_metadata[index];
> +          m_field_metadata[i]= sint2korr(x);
> +          index= index + sizeof(short int);
> +          break;
> +        }
> +        case MYSQL_TYPE_NEWDECIMAL:
> +        {
> +          short int x= field_metadata[index] << 8U; // precision
> +          index++;
> +          x = x + field_metadata[index];            // decimals
> +          m_field_metadata[i]= x;
> +          index++;
>   

See above.

> +          break;
> +        }
> +        default:
> +          m_field_metadata[i]= 0;
> +          break;
> +        }
> +      }
> +    }
> +    if (m_size && null_bitmap)
> +    {
> +       m_null_bits= new uchar[(m_size + 7) / 8];
>   

Use my_multi_malloc() instead, if possible. Many calls to malloc() cause 
threads to caravan.

> +       memcpy(m_null_bits, null_bitmap, (m_size + 7) / 8);
> +    }
>    }
>  
>    ~table_def() {
>      if (m_type)
>        delete [] m_type;
> +    if (m_field_metadata)
> +      delete [] m_field_metadata;
> +    if (m_null_bits)
> +      delete [] m_null_bits;
>   

The "if" part is redundant for "delete[]" since it is well defined that 
deleting NULL pointers are defined to be a NO-OP (this is not critical, 
so do as you like).

>  #ifndef DBUG_OFF
>      m_type= 0;
>      m_size= 0;
> @@ -100,6 +177,47 @@
>      return m_type[index];
>    }
>  
> +  /*
> +    This function allows callers to get the extra field data from the
> +    table map for a given field. If there is no metadata for that field
> +    or there is no extra metadata at all, the function returns 0.
> +
> +    The function returns the value for the field metadata for column at 
> +    position indicated by index. As mentioned, if the field was a type 
> +    that stores field metadata, that value is returned else zero (0) is 
> +    returned. This method is used in the unpack() methods of the 
> +    corresponding fields to properly extract the data from the binary log 
> +    in the event that the master's field is smaller than the slave.
> +  */
> +  uint field_metadata(uint index) const
>   

Since it is 16 bits, the return value should be uint16. The index type 
should be signed, and usually my_ptrdiff_t is used for indexes (we are 
switching to ptrdiff_t though, IIRC).

> +  {
> +    DBUG_ASSERT(index < m_size);
> +    if (m_field_metadata)
> +      return m_field_metadata[index];
> +    else
> +      return 0;
> +  }
> +
> +  /*
> +    This function returns whether the field on the master can be null.
> +    This value is derived from field->maybe_null().
> +  */
> +  my_bool maybe_null(ulong index) const
>   

Use the same type for all indexes, please.

> +  {
> +    DBUG_ASSERT(index < m_size);
> +    return ((m_null_bits[(index / 8)] & 
> +            (1 << (index % 8))) == (1 << (index %8)));
> +  }
> +
> +  /*
> +    This function returns the field size in raw bytes based on the type
> +    and the encoded field data from the master's raw data. This method can 
> +    be used for situations where the slave needs to skip a column (e.g., 
> +    WL#3915) or needs to advance the pointer for the fields in the raw 
> +    data from the master to a specific column.
> +  */
> +  uint32 get_field_size(uint col, uchar *master_data);
> +
>    /**
>      Decide if the table definition is compatible with a table.
>  
> @@ -122,6 +240,8 @@
>  private:
>    my_size_t m_size;           // Number of elements in the types array
>    field_type *m_type;                     // Array of type descriptors
> +  short int *m_field_metadata;
> +  uchar *m_null_bits;
>  };
>  
>  /**
>
> --- 1.2/sql/rpl_record.cc	2007-07-23 17:19:39 -04:00
> +++ 1.3/sql/rpl_record.cc	2007-07-23 17:19:39 -04:00
> @@ -16,6 +16,8 @@
>  #include "mysql_priv.h"
>  #include "rpl_record.h"
>  #include "slave.h"                  // Need to pull in slave_print_msg
> +#include "rpl_utility.h"
> +#include "rpl_rli.h"
>  
>  /**
>     Pack a record of data for a table into a format suitable for
> @@ -190,7 +192,9 @@
>    unsigned int null_mask= 1U;
>    // The "current" null bits
>    unsigned int null_bits= *null_ptr++;
> -  for (field_ptr= begin_ptr ; field_ptr < end_ptr ; ++field_ptr)
> +  uint i= 0;
> +  table_def *tabledef=
> const_cast<RELAY_LOG_INFO*>(rli)->get_tabledef(table);
> +  for (field_ptr= begin_ptr ; field_ptr < end_ptr && *field_ptr ;
> ++field_ptr)
>    {
>      Field *const f= *field_ptr;
>  
> @@ -219,14 +223,20 @@
>          f->set_notnull();
>  
>          /*
> -          We only unpack the field if it was non-null
> -        */
> -        pack_ptr= f->unpack(f->ptr, pack_ptr);
> +          We only unpack the field if it was non-null.
> +          Use the master's size information if available else call
> +          normal unpack operation.
> +         */
> +        if (tabledef && tabledef->field_metadata(i))
> +          pack_ptr= f->unpack(f->ptr, pack_ptr,
> tabledef->field_metadata(i));
> +        else
> +          pack_ptr= f->unpack(f->ptr, pack_ptr);
>        }
>  
>        bitmap_set_bit(rw_set, f->field_index);
>        null_mask <<= 1;
>      }
> +    i++;
>    }
>  
>    /*
>
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com


Thread
bk commit into 5.1 tree (cbell:1.2564)cbell23 Jul
  • Re: bk commit into 5.1 tree (cbell:1.2564)Mats Kindahl25 Jul
    • RE: bk commit into 5.1 tree (cbell:1.2564)Chuck Bell26 Jul
      • RE: bk commit into 5.1 tree (cbell:1.2564)Chuck Bell27 Jul
      • Re: bk commit into 5.1 tree (cbell:1.2564)Mats Kindahl27 Jul