bzr commit into mysql-5.1 branch (mats:2612) Bug#31502

[Mats Kindahl]

#At file:///home/bzr/b31502-mysql-5.1-rpl/

 2612 Mats Kindahl	2008-06-17
      Bug #31502: 5.1.20 -> 5.1.22 Slave crashes if it gets an event
                  w/ data for non-exist column
      
      5.1.16 to 5.1.20 table map event does not contain metadata for
      fields, i.e., there is no information about the parameters for
      the types. When replicating to a 5.1.22 server that expects
      metadata, it turns out to incorrectly be zero in many cases,
      leading to misaligned rows.
      
      This patch does a decent job of providing length of fields
      where the length can be computed without the metadata, and
      throws an error otherwise, stopping the slave.
added:
  mysql-test/suite/bugs/r/rpl_bug31502.result
  mysql-test/suite/bugs/t/rpl_bug31502.test
modified:
  include/my_base.h
  sql/log_event.cc
  sql/log_event.h
  sql/log_event_old.cc
  sql/rpl_record.cc
  sql/rpl_utility.cc
  sql/rpl_utility.h
  sql/share/errmsg.txt

per-file messages:
  include/my_base.h
    Adding new handler-level error code HA_ERR_SLAVE_MISSING_METADATA.
  mysql-test/suite/bugs/r/rpl_bug31502.result
    New result file
  mysql-test/suite/bugs/t/rpl_bug31502.test
    New test file for BUG#31502
  sql/log_event.cc
    Adding error code checking and propagation for unpack_current_row()
    so that a failure in that function will propagate the error upwards.
  sql/log_event.h
    Adding comment on design decision for do_exec_row() and using
    Doxygen comment for it.
  sql/log_event_old.cc
    Adding error code checking and propagation for unpack_current_row()
    so that a failure in that function will propagate the error upwards.
  sql/rpl_record.cc
    Returning error from unpack_row() when row cannot be unpacked due to missing
    parameters (metadata). For some fields, the extra parameter info has to be
    present.
  sql/rpl_utility.cc
    Setting default values for fields when metadata is missing. If
    field size cannot be computed at all, an error is returned.
  sql/rpl_utility.h
    Changing signature of calc_field_data() to be able to return error
    and length.
  sql/share/errmsg.txt
    Adding error message ER_SLAVE_MISSING_PARAMS denoting that
    parameters are missing for some field and the length cannot
    be computed.
=== modified file 'include/my_base.h'
--- a/include/my_base.h	2008-03-28 16:45:03 +0000
+++ b/include/my_base.h	2008-06-17 13:55:53 +0000
@@ -441,7 +441,11 @@ enum ha_base_keytype {
 #define HA_ERR_INITIALIZATION     174    /* Error during initialization */
 #define HA_ERR_FILE_TOO_SHORT	  175	 /* File too short */
 #define HA_ERR_WRONG_CRC	  176	 /* Wrong CRC on page */
-#define HA_ERR_LAST               176    /* Copy of last error nr */
+#define HA_ERR_SLAVE_MISSING_METADATA 177 /* Metadata is missing from
+                                             binary log and is
+                                             required to reconstruct
+                                             row */
+#define HA_ERR_LAST               177    /* Copy of last error nr */
 
 /* Number of different errors */
 #define HA_ERR_ERRORS            (HA_ERR_LAST - HA_ERR_FIRST + 1)

=== added file 'mysql-test/suite/bugs/r/rpl_bug31502.result'
--- a/mysql-test/suite/bugs/r/rpl_bug31502.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/bugs/r/rpl_bug31502.result	2008-06-17 13:55:53 +0000
@@ -0,0 +1,74 @@
+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;
+SET binlog_format = row;
+CREATE TABLE t1 (a CHAR(20), b FLOAT) ENGINE=MyISAM;
+CREATE TABLE t2 (a CHAR(20), b CHAR(64)) ENGINE=MyISAM;
+CREATE TABLE t3 (a CHAR(20), b BLOB) ENGINE=MyISAM;
+ALTER TABLE t1 DROP b;
+ALTER TABLE t2 DROP b;
+ALTER TABLE t3 DROP b;
+INSERT into t1 values ('xxxxxxxxxxx', 1.0);
+SELECT * FROM t1;
+a	b
+xxxxxxxxxxx	1
+SELECT * FROM t1;
+a
+xxxxxxxxxxx
+INSERT into t2 values ('xxxxxxxxxxx', 'blah');
+SELECT * FROM t2;
+a	b
+xxxxxxxxxxx	blah
+SELECT * FROM t2;
+a
+xxxxxxxxxxx
+INSERT into t3 values ('xxxxxxxxxxx', 'This is a blob');
+SELECT * FROM t3;
+a	b
+xxxxxxxxxxx	This is a blob
+SELECT * FROM t3;
+a
+SHOW SLAVE STATUS;
+Slave_IO_State	#
+Master_Host	127.0.0.1
+Master_User	root
+Master_Port	MASTER_PORT
+Connect_Retry	1
+Master_Log_File	master-bin.000001
+Read_Master_Log_Pos	807
+Relay_Log_File	#
+Relay_Log_Pos	#
+Relay_Master_Log_File	master-bin.000001
+Slave_IO_Running	Yes
+Slave_SQL_Running	No
+Replicate_Do_DB	
+Replicate_Ignore_DB	
+Replicate_Do_Table	
+Replicate_Ignore_Table	#
+Replicate_Wild_Do_Table	
+Replicate_Wild_Ignore_Table	
+Last_Errno	1608
+Last_Error	Error in Write_rows event: error during transaction execution on table
test.t3. 
+Skip_Counter	0
+Exec_Master_Log_Pos	709
+Relay_Log_Space	#
+Until_Condition	None
+Until_Log_File	
+Until_Log_Pos	0
+Master_SSL_Allowed	No
+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	No
+Last_IO_Errno	#
+Last_IO_Error	#
+Last_SQL_Errno	1608
+Last_SQL_Error	Error in Write_rows event: error during transaction execution on table
test.t3. 
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER = 1;
+START SLAVE;

=== added file 'mysql-test/suite/bugs/t/rpl_bug31502.test'
--- a/mysql-test/suite/bugs/t/rpl_bug31502.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/bugs/t/rpl_bug31502.test	2008-06-17 13:55:53 +0000
@@ -0,0 +1,75 @@
+# BUG#31502
+#
+#    5.1.20 -> 5.1.22 Slave crashes if it gets an event w/ data for
+#    non-exist column
+
+# Test to check that replication from new format events missing
+# metadata (5.1.16 -- 5.1.20) can replicate in a sensible way to new
+# format events with metadata (5.1.21 and onward).
+
+# In order to run this test, you need to have an old server to
+# replicate from, and use the mysql versional test script
+# runner. There is a full description of how to repeat the failure in
+# the bug report for BUG#31502.
+
+source include/master-slave.inc;
+SET binlog_format = row;
+
+# Clean up from previous tests
+--disable_warnings
+--disable_query_log
+
+DROP TABLE IF EXISTS t1;
+
+--enable_query_log
+--enable_warnings
+
+connection master;
+CREATE TABLE t1 (a CHAR(20), b FLOAT) ENGINE=MyISAM;
+CREATE TABLE t2 (a CHAR(20), b CHAR(64)) ENGINE=MyISAM;
+CREATE TABLE t3 (a CHAR(20), b BLOB) ENGINE=MyISAM;
+
+sync_slave_with_master;
+ALTER TABLE t1 DROP b;
+ALTER TABLE t2 DROP b;
+ALTER TABLE t3 DROP b;
+
+connection master;
+INSERT into t1 values ('xxxxxxxxxxx', 1.0);
+SELECT * FROM t1;
+sync_slave_with_master;
+SELECT * FROM t1;
+
+connection master;
+INSERT into t2 values ('xxxxxxxxxxx', 'blah');
+SELECT * FROM t2;
+sync_slave_with_master;
+SELECT * FROM t2;
+
+connection master;
+INSERT into t3 values ('xxxxxxxxxxx', 'This is a blob');
+SELECT * FROM t3;
+connection slave;
+source include/wait_for_slave_sql_to_stop.inc;
+SELECT * FROM t3;
+source include/show_slave_status.inc;
+
+SET GLOBAL SQL_SLAVE_SKIP_COUNTER = 1;
+START SLAVE;
+source include/wait_for_slave_to_start.inc;
+
+#### Clean Up ####
+
+connection master;
+--disable_warnings
+--disable_query_log
+DROP TABLE t1;
+
+#connection slave;
+sync_slave_with_master;
+--enable_query_log
+--enable_warnings
+
+# END of the tests
+
+

=== modified file 'sql/log_event.cc'
--- a/sql/log_event.cc	2008-05-30 09:12:07 +0000
+++ b/sql/log_event.cc	2008-06-17 13:55:53 +0000
@@ -6649,19 +6649,23 @@ int Rows_log_event::do_apply_event(Relay
       }
 
       /*
-       If m_curr_row_end  was not set during event execution (e.g., because
-       of errors) we can't proceed to the next row. If the error is transient
-       (i.e., error==0 at this point) we must call unpack_current_row() to set 
-       m_curr_row_end.
-      */ 
-   
+       If m_curr_row_end was not set during event execution (e.g.,
+       because of errors) we can't proceed to the next row. If the
+       error is transient (i.e., error==0 at this point) we must call
+       unpack_current_row() to set m_curr_row_end.
+
+       If unpack_current_row() returns an error at this point, we have
+       to abort application of this event with an error.
+      */
+
       DBUG_PRINT("info", ("error: %d", error));
       DBUG_PRINT("info", ("curr_row: 0x%lu; curr_row_end: 0x%lu; rows_end: 0x%lu",
                           (ulong) m_curr_row, (ulong) m_curr_row_end, (ulong)
m_rows_end));
 
       if (!m_curr_row_end && !error)
-        unpack_current_row(rli);
-  
+        if ((error= unpack_current_row(rli)))
+          DBUG_RETURN(error);
+
       // at this moment m_curr_row_end should be set
       DBUG_ASSERT(error || m_curr_row_end != NULL); 
       DBUG_ASSERT(error || m_curr_row < m_curr_row_end);
@@ -7608,9 +7612,15 @@ Rows_log_event::write_row(const Relay_lo
   if ((error= prepare_record(table, m_width,
                              TRUE /* check if columns have def. values */)))
     DBUG_RETURN(error);
-  
-  /* unpack row into table->record[0] */
-  error= unpack_current_row(rli); // TODO: how to handle errors?
+
+  /*
+    Unpack row into table->record[0]
+
+    If unpacking the row failed, we cannot write the row, so we have
+    to abort with an error.
+  */
+  if ((error= unpack_current_row(rli)))
+    DBUG_RETURN(error);
 
 #ifndef DBUG_OFF
   DBUG_DUMP("record[0]", table->record[0], table->s->reclength);
@@ -7713,6 +7723,8 @@ Rows_log_event::write_row(const Relay_lo
     {
       restore_record(table,record[1]);
       error= unpack_current_row(rli);
+      // Since we unpacked the row previously, it should not fail.
+      DBUG_ASSERT(error == 0);
     }
 
 #ifndef DBUG_OFF
@@ -7931,9 +7943,12 @@ int Rows_log_event::find_row(const Relay
     and this is okay with Delete or Update events.
     Todo: fix wl3228 hld that requires defauls for all types of events
   */
-  
+
   prepare_record(table, m_width, FALSE);
-  error= unpack_current_row(rli);
+
+  // If unpacking the row here fails, we cannot proceed.
+  if ((error= unpack_current_row(rli)))
+    DBUG_RETURN(error);
 
 #ifndef DBUG_OFF
   DBUG_PRINT("info",("looking for the following record"));
@@ -8360,9 +8375,14 @@ Update_rows_log_event::do_exec_row(const
     /*
       We need to read the second image in the event of error to be
       able to skip to the next pair of updates
+
+      Don't bother to call unpack_current_row() if the error is
+      HA_ERR_SLAVE_MISSING_METADATA since we will just get the same
+      error back and be in the same position as before the call.
     */
     m_curr_row= m_curr_row_end;
-    unpack_current_row(rli);
+    if (error != HA_ERR_SLAVE_MISSING_METADATA)
+      unpack_current_row(rli);
     return error;
   }
 

=== modified file 'sql/log_event.h'
--- a/sql/log_event.h	2008-03-28 13:52:33 +0000
+++ b/sql/log_event.h	2008-06-17 13:55:53 +0000
@@ -3514,18 +3514,21 @@ private:
   int do_after_row_operations(const Slave_reporting_capability *const log,
                               int error) = 0;
 
-  /*
+  /**
     Primitive to do the actual execution necessary for a row.
 
-    DESCRIPTION
-      The member function will do the actual execution needed to handle a row.
-      The row is located at m_curr_row. When the function returns, 
-      m_curr_row_end should point at the next row (one byte after the end
-      of the current row).    
-
-    RETURN VALUE
-      0 if execution succeeded, 1 if execution failed.
-      
+    The member function will do the actual execution needed to handle
+    a row.  The row is located at @c m_curr_row. When the function
+    returns, @c m_curr_row_end should point at the next row (one byte
+    after the end of the current row).
+
+    @note This function should always return handler-level errors,
+    i.e. those starting with @c HA_ERR. The @c ER_ codes are used in
+    the upper layers, while the handler layer, including the support
+    code used to apply an event, uses handler-level errors.
+
+    @retval 0 if execution succeeded
+    @retval error if execution failed
   */
   virtual int do_exec_row(const Relay_log_info *const rli) = 0;
 #endif /* !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) */

=== modified file 'sql/log_event_old.cc'
--- a/sql/log_event_old.cc	2008-05-12 17:50:53 +0000
+++ b/sql/log_event_old.cc	2008-06-17 13:55:53 +0000
@@ -1714,13 +1714,16 @@ int Old_rows_log_event::do_apply_event(R
                           (ulong) m_curr_row, (ulong) m_curr_row_end, (ulong)
m_rows_end));
 
       if (!m_curr_row_end && !error)
-        unpack_current_row(rli);
-  
-      // at this moment m_curr_row_end should be set
+        error= unpack_current_row(rli);
+
+      /*
+        At this moment m_curr_row_end should be set or we should have
+        an error.
+      */
       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);
-  
+
       m_curr_row= m_curr_row_end;
  
     } // row processing loop
@@ -2046,9 +2049,14 @@ Old_rows_log_event::write_row(const Rela
   if ((error= prepare_record(table, m_width,
                              TRUE /* check if columns have def. values */)))
     DBUG_RETURN(error);
-  
-  /* unpack row into table->record[0] */
-  error= unpack_current_row(rli); // TODO: how to handle errors?
+
+  /*
+    Unpack row into table->record[0]
+
+    If unpacking fails, we cannot proceed, so we return an error.
+  */
+  if ((error= unpack_current_row(rli)))
+    DBUG_RETURN(error);
 
 #ifndef DBUG_OFF
   DBUG_DUMP("record[0]", table->record[0], table->s->reclength);
@@ -2151,7 +2159,8 @@ Old_rows_log_event::write_row(const Rela
     if (!get_flags(COMPLETE_ROWS_F))
     {
       restore_record(table,record[1]);
-      error= unpack_current_row(rli);
+      if ((error= unpack_current_row(rli)))
+        DBUG_RETURN(error);
     }
 
 #ifndef DBUG_OFF
@@ -2233,8 +2242,12 @@ Old_rows_log_event::write_row(const Rela
   can contain extra columns not present in the row. It is also possible that 
   the table has fewer columns than the row being located. 
 
-  @returns Error code on failure, 0 on success. 
-  
+  @retval 0 Success
+
+  @retval HA_ERR_SLAVE_MISSING_METADATA
+  Replication from an old master was attempted, but some columns have
+  types that need metadata to replicate correctly.
+
   @post In case of success @c m_table->record[0] contains the record found. 
   Also, the internal "cursor" of the table is positioned at the record found.
 
@@ -2255,7 +2268,8 @@ int Old_rows_log_event::find_row(const R
 
   // TODO: shall we check and report errors here?
   prepare_record(table, m_width, FALSE /* don't check errors */); 
-  error= unpack_current_row(rli); 
+  if ((error= unpack_current_row(rli)))
+    DBUG_RETURN(error);
 
 #ifndef DBUG_OFF
   DBUG_PRINT("info",("looking for the following record"));
@@ -2818,10 +2832,15 @@ Update_rows_log_event_old::do_exec_row(c
   {
     /*
       We need to read the second image in the event of error to be
-      able to skip to the next pair of updates
+      able to skip to the next pair of updates.
+
+      Don't bother to call the unpack_current_row() if the previous
+      failure was HA_ERR_SLAVE_MISSING_METADATA since we will just get the
+      same error back from the call.
     */
     m_curr_row= m_curr_row_end;
-    unpack_current_row(rli);
+    if (error != HA_ERR_SLAVE_MISSING_METADATA)
+      unpack_current_row(rli);
     return error;
   }
 

=== modified file 'sql/rpl_record.cc'
--- a/sql/rpl_record.cc	2008-02-05 13:52:20 +0000
+++ b/sql/rpl_record.cc	2008-06-17 13:55:53 +0000
@@ -156,6 +156,9 @@ pack_row(TABLE *table, MY_BITMAP const* 
    At most @c colcnt columns are read: if the table is larger than
    that, the remaining fields are not filled in.
 
+   @todo Change error codes returned from this function to always be
+   handler-level errors.
+
    @param rli     Relay log info
    @param table   Table to unpack into
    @param colcnt  Number of columns to read from record
@@ -174,6 +177,9 @@ pack_row(TABLE *table, MY_BITMAP const* 
    Returned if one of the fields existing on the slave but not on the
    master does not have a default value (and isn't nullable)
 
+   @retval HA_ERR_SLAVE_MISSING_METADATA
+   Returned if metadata is missing for one of the extra fields and the
+   size of the field cannot be computed without the metadata.
  */
 #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
 int
@@ -210,6 +216,7 @@ unpack_row(Relay_log_info const *rli,
       No need to bother about columns that does not exist: they have
       gotten default values when being emptied above.
      */
+    DBUG_ASSERT(i == (uint) (field_ptr - begin_ptr));
     if (bitmap_is_set(cols, field_ptr -  begin_ptr))
     {
       if ((null_mask & 0xFF) == 0)
@@ -240,9 +247,9 @@ unpack_row(Relay_log_info const *rli,
         uchar const *const old_pack_ptr= pack_ptr;
 #endif
         pack_ptr= f->unpack(f->ptr, pack_ptr, metadata, TRUE);
-	DBUG_PRINT("debug", ("field: %s; metadata: 0x%x;"
+	DBUG_PRINT("debug", ("field: %s; type: %d; metadata: 0x%x;"
                              " pack_ptr: 0x%lx; pack_ptr': 0x%lx; bytes: %d",
-                             f->field_name, metadata,
+                             f->field_name, f->type(), metadata,
                              (ulong) old_pack_ptr, (ulong) pack_ptr,
                              (int) (pack_ptr - old_pack_ptr)));
       }
@@ -255,6 +262,8 @@ unpack_row(Relay_log_info const *rli,
   /*
     throw away master's extra fields
   */
+  DBUG_PRINT("info", ("i: %u; tabledef->size: %lu; cols->n_bits: %u",
+                      i, tabledef->size(), cols->n_bits));
   uint max_cols= min(tabledef->size(), cols->n_bits);
   for (; i < max_cols; i++)
   {
@@ -268,8 +277,16 @@ unpack_row(Relay_log_info const *rli,
       }
       DBUG_ASSERT(null_mask & 0xFF); // One of the 8 LSB should be set
 
-      if (!((null_bits & null_mask) && tabledef->maybe_null(i)))
-        pack_ptr+= tabledef->calc_field_size(i, (uchar *) pack_ptr);
+      if (!((null_bits & null_mask) && tabledef->maybe_null(i))) {
+        uint32 length;
+        if ((error= tabledef->calc_field_size(i, (uchar *) pack_ptr, &length,
rli)))
+          DBUG_RETURN(error);
+        DBUG_PRINT("info",
+                   ("type: %d; pack_ptr: 0x%lu; pack_ptr': 0x%lu",
+                    tabledef->type(i),
+                    (ulong) pack_ptr, (ulong) pack_ptr + length));
+        pack_ptr += length;
+      }
       null_mask <<= 1;
     }
   }

=== modified file 'sql/rpl_utility.cc'
--- a/sql/rpl_utility.cc	2008-01-30 16:35:25 +0000
+++ b/sql/rpl_utility.cc	2008-06-17 13:55:53 +0000
@@ -20,11 +20,87 @@
  *                   table_def member definitions                    *
  *********************************************************************/
 
-/*
+/**
+   Check and set length of parameter based on metadata length.
+ */
+int
+check_and_set_length(enum_field_types field_type, uint32 length,
+                     Slave_reporting_capability const *log, uint32 *out_length)
+{
+  int error= 0;
+  switch (field_type) {
+  case MYSQL_TYPE_STRING:
+    *out_length = length;
+    break;
+
+  case MYSQL_TYPE_NULL:
+    *out_length = length ? length : 0;
+    break;
+
+  case MYSQL_TYPE_TINY:
+  case MYSQL_TYPE_YEAR:
+    *out_length = length ? length : 1;
+    break;
+
+  case MYSQL_TYPE_SHORT:
+    *out_length = length ? length : 2;
+    break;
+
+  case MYSQL_TYPE_DATE:
+  case MYSQL_TYPE_TIME:
+  case MYSQL_TYPE_NEWDATE:
+  case MYSQL_TYPE_INT24:
+    *out_length = length ? length : 3;
+    break;
+
+  case MYSQL_TYPE_TIMESTAMP:
+  case MYSQL_TYPE_FLOAT:
+  case MYSQL_TYPE_LONG:
+    *out_length = length ? length : 4;
+    break;
+
+  case MYSQL_TYPE_DATETIME:
+  case MYSQL_TYPE_DOUBLE:
+  case MYSQL_TYPE_LONGLONG:
+    *out_length = length ? length : 8;
+    break;
+
+  case MYSQL_TYPE_ENUM:
+  case MYSQL_TYPE_SET:
+  case MYSQL_TYPE_NEWDECIMAL:
+  case MYSQL_TYPE_TINY_BLOB:
+  case MYSQL_TYPE_MEDIUM_BLOB:
+  case MYSQL_TYPE_LONG_BLOB:
+  case MYSQL_TYPE_BLOB:
+  case MYSQL_TYPE_GEOMETRY:
+  case MYSQL_TYPE_VAR_STRING:
+  case MYSQL_TYPE_VARCHAR:
+  case MYSQL_TYPE_BIT:
+  case MYSQL_TYPE_DECIMAL:                      // ???
+    log->report(ERROR_LEVEL, ER_SLAVE_MISSING_METADATA,
+                ER(ER_SLAVE_MISSING_METADATA), field_type);
+    error= HA_ERR_SLAVE_MISSING_METADATA;
+    break;
+  }
+  return error;
+}
+
+
+/**
+  Compute the field size based on the type and the metadata.
+
   This function returns the field size in raw bytes based on the type
-  and the encoded field data from the master's raw data.
+  and the encoded field data from the master's raw data. If the
+  metadata is not present (that is, it's zero), then the slave will do
+  a "best effort" to replicate the data, but throw an error if it is
+  not possible to replicate the data.
+
+  @param len_arg Pointer to variable that will hold the length.
+  @return Error code, or zero if there were no error.
 */
-uint32 table_def::calc_field_size(uint col, uchar *master_data) const
+int
+table_def::calc_field_size(uint col, uchar *master_data, uint32 *len_arg,
+                           Slave_reporting_capability const* log) const
 {
   uint32 length;
 
@@ -34,34 +110,23 @@ uint32 table_def::calc_field_size(uint c
                                        m_field_metadata[col] & 0xff);
     break;
   case MYSQL_TYPE_DECIMAL:
+    length= m_field_metadata[col] & 0x00ff;
+    break;
+
   case MYSQL_TYPE_FLOAT:
   case MYSQL_TYPE_DOUBLE:
-    length= m_field_metadata[col];
-    break;
-  /*
-    The cases for SET and ENUM are include for completeness, however
-    both are mapped to type MYSQL_TYPE_STRING and their real types
-    are encoded in the field metadata.
-  */
   case MYSQL_TYPE_SET:
   case MYSQL_TYPE_ENUM:
+    length= m_field_metadata[col] & 0x00ff;
+    break;
   case MYSQL_TYPE_STRING:
-  {
-    uchar type= m_field_metadata[col] >> 8U;
-    if ((type == MYSQL_TYPE_SET) || (type == MYSQL_TYPE_ENUM))
-      length= m_field_metadata[col] & 0x00ff;
-    else
-    {
-      /*
-        We are reading the actual size from the master_data record
-        because this field has the actual lengh stored in the first
-        byte.
-      */
-      length= (uint) *master_data + 1;
-      DBUG_ASSERT(length != 0);
-    }
+    /*
+      We are reading the actual size from the master_data record
+      because this field has the actual lengh stored in the first
+      byte.
+    */
+    length= (uint) *master_data + 1;
     break;
-  }
   case MYSQL_TYPE_YEAR:
   case MYSQL_TYPE_TINY:
     length= 1;
@@ -113,8 +178,11 @@ uint32 table_def::calc_field_size(uint c
   }
   case MYSQL_TYPE_VARCHAR:
   {
-    length= m_field_metadata[col] > 255 ? 2 : 1; // c&p of
Field_varstring::data_length()
-    DBUG_ASSERT(uint2korr(master_data) > 0);
+    /*
+      This only works when the metadata is 0 if the VARCHAR is less
+      than 255.
+     */
+    length= m_field_metadata[col] > 255 ? 2 : 1;
     length+= length == 1 ? (uint32) *master_data : uint2korr(master_data);
     break;
   }
@@ -166,7 +234,9 @@ uint32 table_def::calc_field_size(uint c
   default:
     length= ~(uint32) 0;
   }
-  return length;
+
+  int const error= check_and_set_length(type(col), length, log, len_arg);
+  return error;
 }
 
 /*

=== modified file 'sql/rpl_utility.h'
--- a/sql/rpl_utility.h	2007-10-01 09:25:32 +0000
+++ b/sql/rpl_utility.h	2008-06-17 13:55:53 +0000
@@ -171,10 +171,10 @@ public:
     <code>index</code>. Currently, only the type identifier is
     returned.
    */
-  field_type type(ulong index) const
+  enum_field_types type(ulong index) const
   {
     DBUG_ASSERT(index < m_size);
-    return m_type[index];
+    return static_cast<enum_field_types>(m_type[index]);
   }
 
 
@@ -216,8 +216,15 @@ public:
     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.
+
+    @param col         Column number
+    @param master_data Pointer to packed field data from master
+    @param len_arg     Pointer to variable holding length if no error
+    @param log         Error logging object
+    @return error code, or zero if no error
   */
-  uint32 calc_field_size(uint col, uchar *master_data) const;
+  int calc_field_size(uint col, uchar *master_data, uint32 *len_arg,
+                      Slave_reporting_capability const* log) const;
 
   /**
     Decide if the table definition is compatible with a table.

=== modified file 'sql/share/errmsg.txt'
--- a/sql/share/errmsg.txt	2008-05-20 16:36:26 +0000
+++ b/sql/share/errmsg.txt	2008-06-17 13:55:53 +0000
@@ -6130,3 +6130,5 @@ ER_LOG_PURGE_NO_FILE  
 
 ER_NEED_REPREPARE
   eng "Prepared statement needs to be re-prepared"
+ER_SLAVE_MISSING_METADATA
+        eng "Field of type %d cannot be reconstructed from master since type parameters
are missing in the binary log.  Use a newer version of the server as master."