Chuck, hi.
Please find some comments inlined.
> ChangeSet@stripped, 2007-07-11 21:23:09-04:00, cbell@mysql_cab_desk. +12 -0
> WL#3228 : 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.
>
> See worklog for additional details.
>
> mysql-test/r/rpl_colSize.result@stripped, 2007-07-11 21:23:05-04:00,
> cbell@mysql_cab_desk. +0 -0
> WL#3228 : prototype test
>
> mysql-test/r/rpl_colSize.result@stripped, 2007-07-11 21:23:05-04:00,
> cbell@mysql_cab_desk. +0 -0
>
> mysql-test/r/rpl_row_max_relay_size.result@stripped, 2007-07-11 21:23:00-04:00,
> cbell@mysql_cab_desk. +10 -10
> WL#3228 : RBR using different table defs on slave/master
>
> This patch changes the test result to coincide with changes to binlog
> size of table map event.
>
> mysql-test/r/rpl_slave_skip.result@stripped, 2007-07-11 21:23:00-04:00,
> cbell@mysql_cab_desk. +2 -2
> WL#3228 : RBR using different table defs on slave/master
>
> This patch changes the test result to coincide with changes to binlog
> size of table map event.
>
> mysql-test/t/rpl_colSize.test@stripped, 2007-07-11 21:23:05-04:00, cbell@mysql_cab_desk.
> +78 -0
> WL#3228 : prototype test
>
> mysql-test/t/rpl_colSize.test@stripped, 2007-07-11 21:23:05-04:00, cbell@mysql_cab_desk.
> +0 -0
>
> sql/field.cc@stripped, 2007-07-11 21:23:01-04:00, cbell@mysql_cab_desk. +105 -3
> 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-11 21:23:01-04:00, cbell@mysql_cab_desk. +13 -1
> 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-11 21:23:02-04:00, cbell@mysql_cab_desk. +152 -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-11 21:23:03-04:00, cbell@mysql_cab_desk. +8 -2
> 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-11 21:23:03-04:00, cbell@mysql_cab_desk. +15 -5
> 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-11 21:23:04-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-11 21:23:04-04:00, cbell@mysql_cab_desk. +82 -33
> 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-11 21:23:04-04:00, cbell@mysql_cab_desk. +80 -4
> 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.
>
> diff -Nrup a/mysql-test/r/rpl_row_max_relay_size.result
> b/mysql-test/r/rpl_row_max_relay_size.result
> --- a/mysql-test/r/rpl_row_max_relay_size.result 2007-06-11 16:15:21 -04:00
> +++ b/mysql-test/r/rpl_row_max_relay_size.result 2007-07-11 21:23:00 -04:00
> @@ -30,7 +30,7 @@ Master_User root
> Master_Port MASTER_PORT
> Connect_Retry 1
> Master_Log_File master-bin.000001
> -Read_Master_Log_Pos 58668
> +Read_Master_Log_Pos 59468
> Relay_Log_File #
> Relay_Log_Pos #
> Relay_Master_Log_File master-bin.000001
> @@ -45,7 +45,7 @@ Replicate_Wild_Ignore_Table
> Last_Errno 0
> Last_Error
> Skip_Counter 0
> -Exec_Master_Log_Pos 58668
> +Exec_Master_Log_Pos 59468
> Relay_Log_Space #
> Until_Condition None
> Until_Log_File
> @@ -78,7 +78,7 @@ Master_User root
> Master_Port MASTER_PORT
> Connect_Retry 1
> Master_Log_File master-bin.000001
> -Read_Master_Log_Pos 58668
> +Read_Master_Log_Pos 59468
> Relay_Log_File #
> Relay_Log_Pos #
> Relay_Master_Log_File master-bin.000001
> @@ -93,7 +93,7 @@ Replicate_Wild_Ignore_Table
> Last_Errno 0
> Last_Error
> Skip_Counter 0
> -Exec_Master_Log_Pos 58668
> +Exec_Master_Log_Pos 59468
> Relay_Log_Space #
> Until_Condition None
> Until_Log_File
> @@ -126,7 +126,7 @@ Master_User root
> Master_Port MASTER_PORT
> Connect_Retry 1
> Master_Log_File master-bin.000001
> -Read_Master_Log_Pos 58668
> +Read_Master_Log_Pos 59468
> Relay_Log_File #
> Relay_Log_Pos #
> Relay_Master_Log_File master-bin.000001
> @@ -141,7 +141,7 @@ Replicate_Wild_Ignore_Table
> Last_Errno 0
> Last_Error
> Skip_Counter 0
> -Exec_Master_Log_Pos 58668
> +Exec_Master_Log_Pos 59468
> Relay_Log_Space #
> Until_Condition None
> Until_Log_File
> @@ -217,7 +217,7 @@ Master_User root
> Master_Port MASTER_PORT
> Connect_Retry 1
> Master_Log_File master-bin.000001
> -Read_Master_Log_Pos 58754
> +Read_Master_Log_Pos 59554
> Relay_Log_File #
> Relay_Log_Pos #
> Relay_Master_Log_File master-bin.000001
> @@ -232,7 +232,7 @@ Replicate_Wild_Ignore_Table
> Last_Errno 0
> Last_Error
> Skip_Counter 0
> -Exec_Master_Log_Pos 58754
> +Exec_Master_Log_Pos 59554
> Relay_Log_Space #
> Until_Condition None
> Until_Log_File
> @@ -261,7 +261,7 @@ Master_User root
> Master_Port MASTER_PORT
> Connect_Retry 1
> Master_Log_File master-bin.000001
> -Read_Master_Log_Pos 58830
> +Read_Master_Log_Pos 59630
> Relay_Log_File #
> Relay_Log_Pos #
> Relay_Master_Log_File master-bin.000001
> @@ -276,7 +276,7 @@ Replicate_Wild_Ignore_Table
> Last_Errno 0
> Last_Error
> Skip_Counter 0
> -Exec_Master_Log_Pos 58830
> +Exec_Master_Log_Pos 59630
> Relay_Log_Space #
> Until_Condition None
> Until_Log_File
> diff -Nrup a/mysql-test/r/rpl_slave_skip.result b/mysql-test/r/rpl_slave_skip.result
> --- a/mysql-test/r/rpl_slave_skip.result 2007-06-13 09:16:26 -04:00
> +++ b/mysql-test/r/rpl_slave_skip.result 2007-07-11 21:23:00 -04:00
> @@ -44,7 +44,7 @@ Master_User root
> Master_Port MASTER_PORT
> Connect_Retry 1
> Master_Log_File master-bin.000001
> -Read_Master_Log_Pos 714
> +Read_Master_Log_Pos 718
> Relay_Log_File #
> Relay_Log_Pos #
> Relay_Master_Log_File master-bin.000001
> @@ -59,7 +59,7 @@ Replicate_Wild_Ignore_Table
> Last_Errno 0
> Last_Error
> Skip_Counter 0
> -Exec_Master_Log_Pos 484
> +Exec_Master_Log_Pos 486
> Relay_Log_Space #
> Until_Condition Master
> Until_Log_File master-bin.000001
> diff -Nrup a/mysql-test/t/rpl_colSize.test b/mysql-test/t/rpl_colSize.test
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/t/rpl_colSize.test 2007-07-11 21:23:05 -04:00
> @@ -0,0 +1,78 @@
> +-- 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 *** 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 ('0','1'),
> + 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')
> +);
> +
> +--echo *** Create t1 on Master ***
> +connection master;
> +eval CREATE TABLE t1 (
> + a float (44),
> + b double (10,3),
> + c decimal (10,2),
> + d numeric (3,0),
> + e bit (20),
> + f char (10),
> + g varchar (100),
> + h binary (7),
> + j varbinary (20),
> + k enum ('0'),
> + l set ('1','2','3','4','5','6','7','8','9','0')
> +);
> +
> +RESET MASTER;
> +
> +--echo *** Start Slave ***
> +connection slave;
> +START SLAVE;
> +
> +--echo *** Master Data Insert ***
> +connection master;
> +
> +INSERT INTO t1 () VALUES (
> + 17.567,
> + 2.123,
> + 10.20,
> + 125,
> + 3,
> + 'TEST',
> + 'This is a test',
> + 'binary data',
> + 'more binary data',
> + '0',
> + '0');
> +
> +SELECT * FROM t1 ORDER BY a;
> +
> +--echo *** Select from slave ***
> +sync_slave_with_master;
> +SELECT * FROM t1 ORDER BY a;
> +
> +--echo *** Drop t1 ***
> +connection master;
> +DROP TABLE t1;
> +sync_slave_with_master;
> +
> +# END 5.1 Test Case
> diff -Nrup a/sql/field.cc b/sql/field.cc
> --- a/sql/field.cc 2007-06-21 11:12:54 -04:00
> +++ b/sql/field.cc 2007-07-11 21:23:01 -04:00
> @@ -2642,6 +2642,31 @@ uint Field_new_decimal::is_equal(Create_
> (new_field->decimals == dec));
> }
>
> +const uchar *Field_new_decimal::unpack(uchar* to, const uchar *from, uint
> from_length)
> +{
> + uint from_precision= from_length >> 8U;
> + uint from_decimal= from_length - (from_precision << 8U);
> + 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;
> + if (from_length && (from_pack_len < length))
> + {
> + decimal_digit_t dec_buf[DECIMAL_MAX_PRECISION];
> + decimal_t dec;
> + dec.len= from_precision;
> + dec.buf= dec_buf;
> + double dbl;
> + bin2decimal((uchar*) from, &dec, from_precision, from_decimal);
> + decimal2double(&dec, &dbl);
> + double2decimal(dbl, &dec);
> + decimal2bin(&dec, to, precision, decimals());
> + printf ("%f %d.%d\n", dbl, precision, decimals());
> + }
> + else
> + memcpy(to, from, len);
> + return from+len;
> +}
>
> /****************************************************************************
> ** tiny int
> @@ -6291,6 +6316,27 @@ uchar *Field_string::pack(uchar *to, con
> }
>
>
> +/*
> + 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 uchar *Field_string::unpack(uchar *to, const uchar *from, uint from_length)
> +{
> + uint length= 0;
> + uint f_length= (from_length && (from_length < field_length)) ?
> + from_length : field_length;
> + if (f_length > 255)
> + {
> + length= uint2korr(from);
> + from+= 2;
> + }
> + else
> + length= (uint) *from++;
> + memcpy(to, from, (int) length);
> + bfill(to+length, field_length - length, ' ');
> + return from+length;
> +}
> +
> const uchar *Field_string::unpack(uchar *to, const uchar *from)
> {
> uint length;
> @@ -6306,7 +6352,6 @@ const uchar *Field_string::unpack(uchar
> return from+length;
> }
>
> -
> /*
> Compare two packed keys
>
> @@ -6783,7 +6828,34 @@ uchar *Field_varstring::pack_key_from_ke
>
> /*
> unpack field packed with Field_varstring::pack()
> +
> + 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 uchar *Field_varstring::unpack(uchar *to, const uchar *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;
> +}
>
> const uchar *Field_varstring::unpack(uchar *to, const uchar *from)
> {
> @@ -6801,7 +6873,6 @@ const uchar *Field_varstring::unpack(uch
> return from+length;
> }
>
> -
> int Field_varstring::pack_cmp(const uchar *a, const uchar *b,
> uint key_length_arg,
> my_bool insert_or_update)
> @@ -7476,6 +7547,14 @@ uchar *Field_blob::pack(uchar *to, const
> return to+packlength+length;
> }
>
> +/*
> + Note: from_length included to satisfy inheritance rules, but is not needed
> + for blob fields.
> +*/
> +const uchar *Field_blob::unpack(uchar *to, const uchar *from, uint from_length)
> +{
> + return unpack(to, from);
> +}
>
> const uchar *Field_blob::unpack(uchar *to, const uchar *from)
> {
> @@ -8524,6 +8603,30 @@ uchar *Field_bit::pack(uchar *to, const
> }
>
>
> +/*
> + 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 uchar *Field_bit::unpack(uchar *to, const uchar *from, uint from_length)
> +{
> + uint from_bit_len= from_length >> 8U;
> + uint from_bytes_in_rec= from_length - (from_bit_len << 8U);
> + if (from_bit_len > 0)
> + {
> + /*
> + set_rec_bits is a macro, don't put the post-increment in the
> + argument since that might cause strange side-effects.
> +
> + For the choice of the second argument, see the explanation for
> + Field_bit::pack().
> + */
> + set_rec_bits(*from, bit_ptr + (to - ptr), bit_ofs, from_bit_len);
> + from++;
> + }
> + memcpy(to, from, from_bytes_in_rec);
> + return from + from_bytes_in_rec;
> +}
> +
> const uchar *Field_bit::unpack(uchar *to, const uchar *from)
> {
> if (bit_len > 0)
> @@ -8541,7 +8644,6 @@ const uchar *Field_bit::unpack(uchar *to
> memcpy(to, from, bytes_in_rec);
> return from + bytes_in_rec;
> }
> -
>
> void Field_bit::set_default()
> {
> diff -Nrup a/sql/field.h b/sql/field.h
> --- a/sql/field.h 2007-06-15 13:36:14 -04:00
> +++ b/sql/field.h 2007-07-11 21:23:01 -04:00
> @@ -278,7 +278,6 @@ public:
> inline void set_image(const uchar *buff,uint length, CHARSET_INFO *cs)
> { memcpy(ptr,buff,length); }
>
> -
> /*
> Copy a field part into an output buffer.
>
> @@ -343,6 +342,14 @@ public:
> memcpy(to,from,length);
> return to+length;
> }
> + virtual const uchar *unpack(uchar* to, const uchar *from, uint from_length)
> + {
> + uint length=pack_length();
> + uint len= (from_length && (from_length < length)) ?
> + from_length : length;
> + memcpy(to,from,len);
> + return from+len;
> + }
Shall we DBUG_ASSERT(from_lenght == 0 || from_length <= length)?
Either unpack with the basic 2-args method or when from_length is
informal it must not exceed the basic's pack_length. In other words
it's good to have a guard against overrun.
> virtual const uchar *unpack(uchar* to, const uchar *from)
> {
> uint length=pack_length();
> @@ -626,6 +633,7 @@ public:
> uint size_of() const { return sizeof(*this); }
> uint32 pack_length() const { return (uint32) bin_size; }
> uint is_equal(Create_field *new_field);
> + const uchar *unpack(uchar* to, const uchar *from, uint from_length);
> };
>
>
> @@ -1162,6 +1170,7 @@ public:
> void sort_string(uchar *buff,uint length);
> void sql_type(String &str) const;
> uchar *pack(uchar *to, const uchar *from, uint max_length=~(uint) 0);
> + const uchar *unpack(uchar* to, const uchar *from, uint from_length);
> const uchar *unpack(uchar* to, const uchar *from);
> int pack_cmp(const uchar *a,const uchar *b,uint key_length,
> my_bool insert_or_update);
> @@ -1238,6 +1247,7 @@ public:
> uchar *pack_key(uchar *to, const uchar *from, uint max_length);
> uchar *pack_key_from_key_image(uchar* to, const uchar *from,
> uint max_length);
> + const uchar *unpack(uchar* to, const uchar *from, uint from_length);
> const uchar *unpack(uchar* to, const uchar *from);
> const uchar *unpack_key(uchar* to, const uchar *from, uint max_length);
> int pack_cmp(const uchar *a, const uchar *b, uint key_length,
> @@ -1377,6 +1387,7 @@ public:
> uchar *pack_key(uchar *to, const uchar *from, uint max_length);
> uchar *pack_key_from_key_image(uchar* to, const uchar *from,
> uint max_length);
> + const uchar *unpack(uchar *to, const uchar *from, uint from_length);
> const uchar *unpack(uchar *to, const uchar *from);
> const uchar *unpack_key(uchar* to, const uchar *from, uint max_length);
> int pack_cmp(const uchar *a, const uchar *b, uint key_length,
> @@ -1552,6 +1563,7 @@ public:
> uint32 pack_length_in_rec() const { return bytes_in_rec; }
> void sql_type(String &str) const;
> uchar *pack(uchar *to, const uchar *from, uint max_length=~(uint) 0);
> + const uchar *unpack(uchar* to, const uchar *from, uint from_length);
> const uchar *unpack(uchar* to, const uchar *from);
> virtual void set_default();
>
> diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
> --- a/sql/log_event.cc 2007-06-28 09:07:53 -04:00
> +++ b/sql/log_event.cc 2007-07-11 21:23:02 -04:00
> @@ -6023,11 +6023,8 @@ int Rows_log_event::do_apply_event(RELAY
> #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);
> -
> TABLE* table=
> const_cast<RELAY_LOG_INFO*>(rli)->m_table_map.get_table(m_table_id);
>
> if (table)
> @@ -6122,6 +6119,15 @@ int Rows_log_event::do_apply_event(RELAY
> }
> }
>
> + /*
> + 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)
> + const_cast<RELAY_LOG_INFO*>(rli)->clear_tables_to_lock();
> +
> + DBUG_ASSERT(rli->tables_to_lock == NULL && rli->tables_to_lock_count
> == 0);
> +
> if (error)
> { /* error has occured during the transaction */
> rli->report(ERROR_LEVEL, thd->net.last_errno,
> @@ -6370,6 +6376,112 @@ void Rows_log_event::print_helper(FILE *
> /**************************************************************************
> 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.
> + */
> +#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_DOUBLE:
> + case MYSQL_TYPE_FLOAT:
> + {
> + size++; // Store one byte here.
> + break;
> + }
> + case MYSQL_TYPE_NEWDECIMAL:
> + case MYSQL_TYPE_ENUM:
> + case MYSQL_TYPE_STRING:
> + case MYSQL_TYPE_VARCHAR:
> + case MYSQL_TYPE_SET:
> + case MYSQL_TYPE_BIT:
> + {
> + 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.",
> + m_field_metadata_size));
> + DBUG_RETURN(m_field_metadata_size);
> +}
> +#endif /* !defined(MYSQL_CLIENT) */
> +
> +/**
> + * 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.
> + */
> +#if !defined(MYSQL_CLIENT)
> +int Table_map_log_event::get_field_metadata()
> +{
> + 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()) {
> + case MYSQL_TYPE_NEWDECIMAL:
> + {
> + m_field_metadata[index]=
> + (byte)((Field_new_decimal *)m_table->s->field[i])->precision;
> + index++;
Imo although
m_field_metadata[index++]
is less readable but is safer
as no chances to forget incrementing of index.
> + m_field_metadata[index]=
> + (byte)((Field_new_decimal *)m_table->s->field[i])->decimals();
> + index++;
> + break;
> + }
> + case MYSQL_TYPE_DOUBLE:
> + case MYSQL_TYPE_FLOAT:
> + {
> + m_field_metadata[index]= (byte)m_table->s->field[i]->pack_length();
> + index++;
> + break;
> + }
> + case MYSQL_TYPE_BIT:
> + {
> + m_field_metadata[index]=
> + (byte)((Field_bit *)m_table->s->field[i])->bit_len;
> + index++;
> + m_field_metadata[index]=
> + (byte)((Field_bit *)m_table->s->field[i])->bytes_in_rec;
> + index++;
> + break;
> + }
> + case MYSQL_TYPE_ENUM:
> + case MYSQL_TYPE_STRING:
> + 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_SET:
> + {
> + short int *x= (short int *)&m_field_metadata[index];
> + int2store(x, m_table->s->field[i]->pack_length());
> + index= index + sizeof(short int);
> + break;
> + }
> + default:
> + break;
> + }
> + }
> + DBUG_RETURN(0);
> +}
> +#endif /* !defined(MYSQL_CLIENT) */
>
> /*
> Constructor used to build an event for writing to the binary log.
> @@ -6385,7 +6497,7 @@ Table_map_log_event::Table_map_log_event
> 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_colcnt(tbl->s->fields), m_field_metadata(0),
> m_table_id(tid),
> m_flags(flags)
> {
> @@ -6406,6 +6518,8 @@ Table_map_log_event::Table_map_log_event
> 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();
> + m_data_size+= m_field_metadata_size + 1;
Suggest to comment: +1 is due to net_store_length-ing of
m_field_metadata_size, right?
Then it might not be enough as merely the max number of columns is more than
256 (I met a link with "MySQL: Maximum number of columns in one table - 3398").
To multiply the max on 2 or even 4 still keeps the needed size within 2 bytes.
>
> /* If malloc fails, catched in is_valid() */
> if ((m_memory= (uchar*) my_malloc(m_colcnt, MYF(MY_WME))))
> @@ -6414,6 +6528,12 @@ Table_map_log_event::Table_map_log_event
> for (unsigned int i= 0 ; i < m_table->s->fields ; ++i)
> m_coltype[i]= m_table->field[i]->type();
> }
> +
> + /*
> + Create an array for the field metadata and store it.
> + */
> + m_field_metadata= new byte[m_field_metadata_size];
> + get_field_metadata();
> }
> #endif /* !defined(MYSQL_CLIENT) */
>
> @@ -6429,8 +6549,10 @@ Table_map_log_event::Table_map_log_event
> #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;
> @@ -6503,12 +6625,24 @@ Table_map_log_event::Table_map_log_event
> 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 byte[m_field_metadata_size];
> + memcpy(m_field_metadata, ptr_after_colcnt, m_field_metadata_size);
> + }
> +
> DBUG_VOID_RETURN;
> }
> #endif
>
> Table_map_log_event::~Table_map_log_event()
> {
> + delete[] m_field_metadata;
> my_free(m_memory, MYF(MY_ALLOW_ZERO_PTR));
> }
>
> @@ -6639,7 +6773,9 @@ int Table_map_log_event::do_apply_event(
> 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);
> +
> table_list->m_tabledef_valid= TRUE;
>
> /*
> @@ -6653,7 +6789,7 @@ int Table_map_log_event::do_apply_event(
> }
>
> DBUG_RETURN(error);
> -
> +
> err:
> my_free(memory, MYF(MY_WME));
> DBUG_RETURN(error);
> @@ -6711,12 +6847,20 @@ bool Table_map_log_event::write_data_bod
> uchar *const cbuf_end= net_store_length(cbuf, (size_t) m_colcnt);
> DBUG_ASSERT(static_cast<size_t>(cbuf_end - cbuf) <= sizeof(cbuf));
>
> + /*
> + Store the size of the field metadata.
> + */
> + uchar mbuf[sizeof(m_field_metadata_size)];
> + uchar *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 uchar*)m_dbnam, m_dblen+1) ||
> my_b_safe_write(file, tbuf, sizeof(tbuf)) ||
> my_b_safe_write(file, (const uchar*)m_tblnam, m_tbllen+1) ||
> my_b_safe_write(file, cbuf, (size_t) (cbuf_end - cbuf)) ||
> - my_b_safe_write(file, m_coltype, m_colcnt));
> + my_b_safe_write(file, m_coltype, m_colcnt) ||
> + my_b_safe_write(file, mbuf, (size_t) (mbuf_end - mbuf)) ||
> + my_b_safe_write(file, m_field_metadata, m_field_metadata_size));
> }
> #endif
>
> diff -Nrup a/sql/log_event.h b/sql/log_event.h
> --- a/sql/log_event.h 2007-06-04 19:15:00 -04:00
> +++ b/sql/log_event.h 2007-07-11 21:23:03 -04:00
> @@ -2049,6 +2049,8 @@ public:
>
> virtual int get_data_size() { return m_data_size; }
> #ifndef MYSQL_CLIENT
> + virtual int get_field_metadata_size();
> + 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; }
> @@ -2079,12 +2081,16 @@ private:
> size_t m_tbllen;
> ulong m_colcnt;
> uchar *m_coltype;
> -
> uchar *m_memory;
> ulong m_table_id;
> flag_set m_flags;
> -
(and of course not to forget on consmetics: usually reviewers ask not to touch unnecessary
lines ... Could you
restore the blank lines above (and there are some places across the
patch) and remove blanks added ?)
> 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;
> };
>
>
> diff -Nrup a/sql/rpl_record.cc b/sql/rpl_record.cc
> --- a/sql/rpl_record.cc 2007-06-11 16:15:28 -04:00
> +++ b/sql/rpl_record.cc 2007-07-11 21:23:03 -04:00
> @@ -17,6 +17,7 @@
> #include "rpl_rli.h"
> #include "rpl_record.h"
> #include "slave.h" // Need to pull in slave_print_msg
> +#include "rpl_utility.h"
>
> /**
> Pack a record of data for a table into a format suitable for
> @@ -124,7 +125,6 @@ pack_row(TABLE *table, MY_BITMAP const*
> }
> #endif
>
> -
> /**
> Unpack a row into @c table->record[0].
>
> @@ -169,6 +169,8 @@ unpack_row(RELAY_LOG_INFO const *rli,
> uchar const **const row_end, ulong *const master_reclength,
> MY_BITMAP* const rw_set, Log_event_type const event_type)
> {
> + my_bool stop= FALSE;
> +
We spoke about `stop' that's for wl#3915 ie for me, thanks!
<started askin before found the mail with a prototype code for wl3915>
it's not changed anymore in the following. Why? Just for testing?
> DBUG_ENTER("unpack_row");
> DBUG_ASSERT(row_data);
> size_t const master_null_byte_count= (bitmap_bits_set(cols) + 7) / 8;
> @@ -191,10 +193,11 @@ unpack_row(RELAY_LOG_INFO const *rli,
> 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)
> + int i= 0;
<started askin>
Another cosmetics wrt to i counter. I'd rather moved initialization in
increment into for (i=0,...i++) as i is incremented per each loop and
that would designate it's not needed outside of it.
> + table_def *tabledef=
> const_cast<RELAY_LOG_INFO*>(rli)->get_tabledef(table);
> + for (field_ptr= begin_ptr ; field_ptr < end_ptr && !stop ;
> ++field_ptr)
> {
> Field *const f= *field_ptr;
> -
> /*
> No need to bother about columns that does not exist: they have
> gotten default values when being emptied above.
> @@ -220,14 +223,21 @@ unpack_row(RELAY_LOG_INFO const *rli,
> f->set_notnull();
>
> /*
> - We only unpack the field if it was non-null
> + We only unpack the field if it was non-null.
> + Use the master's size information if available else call
> + normal unpack operation.
> */
> - pack_ptr= f->unpack(f->ptr, pack_ptr);
> + if (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++;
> }
> +
> }
>
> /*
> diff -Nrup a/sql/rpl_rli.h b/sql/rpl_rli.h
> --- a/sql/rpl_rli.h 2007-06-11 16:15:29 -04:00
> +++ b/sql/rpl_rli.h 2007-07-11 21:23:04 -04:00
> @@ -18,6 +18,7 @@
>
> #include "rpl_tblmap.h"
> #include "rpl_reporting.h"
> +#include "rpl_utility.h"
>
> struct RPL_TABLE_LIST;
>
> @@ -300,6 +301,15 @@ typedef struct st_relay_log_info : publi
> 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);
> + }
>
Don't have a good solution for get_tabledef which is to be called per
row.
Computation expence grows with when number of tables. One day we
would like to optimize that...
One idea is to build a table array to fill with the list's
entries. If the list order coincide with the order of Rows_log events'
tables it'd be enough to compare the current array's index or - when all
rows are done for the current table - to switch to the next.
> /*
> Last charset (6 bytes) seen by slave SQL thread is cached here; it helps
> diff -Nrup a/sql/rpl_utility.cc b/sql/rpl_utility.cc
> --- a/sql/rpl_utility.cc 2007-06-11 16:15:29 -04:00
> +++ b/sql/rpl_utility.cc 2007-07-11 21:23:04 -04:00
> @@ -16,16 +16,72 @@
> #include "rpl_utility.h"
> #include "rpl_rli.h"
>
> -uint32
> -field_length_from_packed(enum_field_types const field_type,
> - uchar const *const data)
> +/*
> + This method gets the field metadata as pertaining to how the field
> + is stored in the raw data from the master. This can be used to determine
> + if the sizes are compatible in either max size (length) or if the
> + packed size is compatible.
> +*/
> +uint get_field_metadata (uint32 type, int col, TABLE *table)
> +{
> + uint field_size= 0;
> + switch (type) {
> + case MYSQL_TYPE_NEWDECIMAL:
> + {
> + field_size= (byte)table->field[col]->pack_length() << 8U;
> + field_size= field_size + ((Field_new_decimal
> *)table->field[col])->decimals();
> + break;
> + }
> + case MYSQL_TYPE_DOUBLE:
> + case MYSQL_TYPE_FLOAT:
> + {
> + field_size= table->field[col]->pack_length();
> + break;
> + }
> + case MYSQL_TYPE_BIT:
> + {
> + field_size= ((Field_bit *)table->field[col])->bit_len << 8U;
> + field_size= field_size + ((Field_bit *)table->field[col])->bytes_in_rec;
> + break;
> + }
> + case MYSQL_TYPE_SET:
> + case MYSQL_TYPE_STRING:
> + case MYSQL_TYPE_ENUM:
> + case MYSQL_TYPE_VARCHAR:
> + {
> + field_size= table->field[col]->field_length;
> + break;
> + }
> + default:
> + field_size= 0;
> + }
> + return (field_size);
> +}
> +
> +/*********************************************************************
> + * 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:
> + case MYSQL_TYPE_SET:
> + case MYSQL_TYPE_ENUM:
> + case MYSQL_TYPE_STRING:
> + length= m_field_metadata[col];
> break;
> case MYSQL_TYPE_YEAR:
> case MYSQL_TYPE_TINY:
> @@ -45,12 +101,6 @@ field_length_from_packed(enum_field_type
> 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;
> @@ -69,41 +119,31 @@ field_length_from_packed(enum_field_type
> 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);
> + length= (m_field_metadata[col] << 8U) >> 8U;
> 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] + m_field_metadata[col] > 255 ? 2 : 1;
> 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], TRUE, "temp",
> system_charset_info);
> + fb->ptr= master_data;
> + length= fb->pack_length();
> + delete fb;
> break;
> }
> -
> + default:
> + length= -1;
> + }
> + printf("Calculated field size of col %d is %d\n", col, length);
> return length;
> }
>
> -/*********************************************************************
> - * table_def member definitions *
> - *********************************************************************/
> -
> /*
> Is the definition compatible with a table?
>
> @@ -112,6 +152,9 @@ int
> table_def::compatible_with(RELAY_LOG_INFO const *rli_arg, TABLE *table)
> const
> {
> + uint received_size= 0;
> + uint field_size= 0;
> +
> /*
> We only check the initial columns for the tables.
> */
> @@ -138,8 +181,14 @@ table_def::compatible_with(RELAY_LOG_INF
> ER(ER_BINLOG_ROW_WRONG_TABLE_DEF), buf);
> }
>
> + /*
> + 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);
> diff -Nrup a/sql/rpl_utility.h b/sql/rpl_utility.h
> --- a/sql/rpl_utility.h 2007-05-23 18:39:21 -04:00
> +++ b/sql/rpl_utility.h 2007-07-11 21:23:04 -04:00
> @@ -25,8 +25,7 @@
> struct st_relay_log_info;
> typedef st_relay_log_info RELAY_LOG_INFO;
>
> -uint32
> -field_length_from_packed(enum_field_types field_type, uchar const *data);
> +uint get_field_metadata (uint32 type, int col, TABLE *table);
>
> /**
> A table definition from the master.
> @@ -58,18 +57,74 @@ public:
> @param types Array of types
> @param size Number of elements in array 'types'
> */
> - table_def(field_type *types, ulong size)
> - : m_size(size), m_type(new unsigned char [size])
> + table_def(field_type *types, ulong size, byte *field_metadata,
> + int metadata_size)
> + : m_size(size), m_type(new unsigned char [size]), m_field_metadata(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
> + 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];
> + int index= 0;
> + for (unsigned int i= 0; i < m_size; i++)
> + {
> + switch (m_type[i]) {
> + case MYSQL_TYPE_DOUBLE:
> + case MYSQL_TYPE_FLOAT:
> + {
> + /*
> + These types store a single byte.
> + */
> + m_field_metadata[i]= (byte)field_metadata[index];
> + index++;
> + break;
> + }
> + case MYSQL_TYPE_SET:
> + case MYSQL_TYPE_ENUM:
> + case MYSQL_TYPE_STRING:
> + 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:
> + case MYSQL_TYPE_BIT:
> + {
> + short int x= field_metadata[index] << 8U; // bit_len
> + index++;
> + x = x + field_metadata[index];
> + m_field_metadata[i]= x;
> + index++;
> + break;
> + }
> + default:
> + m_field_metadata[i]= 0;
> + break;
> + }
> + }
> + }
> }
>
> ~table_def() {
> if (m_type)
> delete [] m_type;
> + if (m_field_metadata)
> + delete [] m_field_metadata;
> #ifndef DBUG_OFF
> m_type= 0;
> m_size= 0;
> @@ -99,6 +154,26 @@ public:
> 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.
> + */
> + uint field_metadata(uint index) const
> + {
> + DBUG_ASSERT(index < m_size);
> + if (m_field_metadata)
> + return m_field_metadata[index];
> + else
> + return 0;
> + }
> +
> + /*
> + 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 get_field_size(uint col, uchar *master_data);
> +
> /**
> Decide if the table definition is compatible with a table.
>
> @@ -121,6 +196,7 @@ public:
> private:
> ulong m_size; // Number of elements in the types array
> field_type *m_type; // Array of type descriptors
> + short int *m_field_metadata;
> };
>
> /**
>
>
Besides the code in the patch I was thinking on emulation of old ->
new replication.
There is a DBUG_EXECUTE_IF() examples left by Mats probably to emulate
a similar scenario when table_map event got changed.
regards,
Andrei
