MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Mats Kindahl Date:December 14 2006 4:23am
Subject:bk commit into 5.1 tree (mats:1.2308) BUG#22583
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of mats. When mats does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2006-12-14 05:23:31+01:00, mats@romeo.(none) +6 -0
  BUG#22583 (RBR between MyISAM and non-MyISAM tables containing BIT field
  does not work): Changing packed row format to only include null bits
  for those columns that are present in the row as well as writing BIT
  columns in a storage engine-independent format.
  
  The change in row format is incompatible with the previous format and a
  slave will not be able to read the new events.

  mysql-test/r/rpl_row_basic_11bugs.result@stripped, 2006-12-14 05:23:24+01:00, mats@romeo.(none) +99 -0
    Result change

  mysql-test/t/rpl_row_basic_11bugs-master.opt@stripped, 2006-12-14 05:23:24+01:00, mats@romeo.(none) +2 -1
    Adding --innodb option

  mysql-test/t/rpl_row_basic_11bugs-slave.opt@stripped, 2006-12-14 05:23:25+01:00, mats@romeo.(none) +1 -0
    New BitKeeper file ``mysql-test/t/rpl_row_basic_11bugs-slave.opt''

  mysql-test/t/rpl_row_basic_11bugs-slave.opt@stripped, 2006-12-14 05:23:25+01:00, mats@romeo.(none) +0 -0

  mysql-test/t/rpl_row_basic_11bugs.test@stripped, 2006-12-14 05:23:24+01:00, mats@romeo.(none) +68 -0
    Testing explicitly for RBR MyISAM -> InnoDB and vice versa.

  sql/log_event.cc@stripped, 2006-12-14 05:23:25+01:00, mats@romeo.(none) +59 -50
    Changing packed row format to only include null bits for those columns
    that are present in the row as well as writing BIT columns in a storage
    engine-independent format.
    
    Changing unpack_row() to accomodate for the changes.

  sql/sql_class.cc@stripped, 2006-12-14 05:23:25+01:00, mats@romeo.(none) +93 -13
    Changing packed row format to only include null bits for those columns
    that are present in the row as well as writing BIT columns in a storage
    engine-independent format.
    
    Changing THD::pack_row() to accomodate for the changes and adding
    documentation.

# 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:	mats
# Host:	romeo.(none)
# Root:	/home/bk/b22583-mysql-5.1-new-rpl

--- 1.246/sql/log_event.cc	2006-12-14 05:23:39 +01:00
+++ 1.247/sql/log_event.cc	2006-12-14 05:23:39 +01:00
@@ -5449,19 +5449,19 @@
   
   SYNOPSIS
     unpack_row()
-    rli     Relay log info
-    table   Table to unpack into
-    colcnt  Number of columns to read from record
-    record  Record where the data should be unpacked
-    row     Packed row data
-    cols    Pointer to columns data to fill in
-    row_end Pointer to variable that will hold the value of the
-            one-after-end position for the row
+    rli      Relay log info
+    table    Table to unpack into
+    colcnt   Number of columns to read from record
+    record   Record where the data should be unpacked
+    row_data Packed row data
+    cols     Pointer to columns data to fill in
+    row_end  Pointer to variable that will hold the value of the
+             one-after-end position for the row
     master_reclength
-            Pointer to variable that will be set to the length of the
-            record on the master side
-    rw_set  Pointer to bitmap that holds either the read_set or the
-            write_set of the table
+             Pointer to variable that will be set to the length of the
+             record on the master side
+    rw_set   Pointer to bitmap that holds either the read_set or the
+             write_set of the table
 
   DESCRIPTION
 
@@ -5484,62 +5484,75 @@
 static int
 unpack_row(RELAY_LOG_INFO *rli,
            TABLE *table, uint const colcnt, byte *record,
-           char const *row, MY_BITMAP const *cols,
-           char const **row_end, ulong *master_reclength,
+           char const *const row_data, MY_BITMAP const *cols,
+           char const **const row_end, ulong *const master_reclength,
            MY_BITMAP* const rw_set, Log_event_type const event_type)
 {
-  DBUG_ASSERT(record && row);
+  DBUG_ASSERT(record && row_data);
   my_ptrdiff_t const offset= record - (byte*) table->record[0];
-  my_size_t master_null_bytes= table->s->null_bytes;
+  my_size_t const master_null_byte_count= (bitmap_bits_set(cols) + 7) / 8;
+  int error= 0;
 
-  if (colcnt != table->s->fields)
-  {
-    Field **fptr= &table->field[colcnt-1];
-    do
-      master_null_bytes= (*fptr)->last_null_byte();
-    while (master_null_bytes == Field::LAST_NULL_BYTE_UNDEF &&
-           fptr-- > table->field);
-
-    /*
-      If master_null_bytes is LAST_NULL_BYTE_UNDEF (0) at this time,
-      there were no nullable fields nor BIT fields at all in the
-      columns that are common to the master and the slave. In that
-      case, there is only one null byte holding the X bit.
-
-      OBSERVE! There might still be nullable columns following the
-      common columns, so table->s->null_bytes might be greater than 1.
-     */
-    if (master_null_bytes == Field::LAST_NULL_BYTE_UNDEF)
-      master_null_bytes= 1;
-  }
+  char const *null_ptr= row_data;
+  char const *pack_ptr= row_data + master_null_byte_count;
 
-  DBUG_ASSERT(master_null_bytes <= table->s->null_bytes);
-  memcpy(record, row, master_null_bytes);            // [1]
-  int error= 0;
+  bitmap_clear_all(rw_set);
 
-  bitmap_set_all(rw_set);
+  memcpy(record, table->s->default_values, table->s->null_bytes);
 
   Field **const begin_ptr = table->field;
   Field **field_ptr;
-  char const *ptr= row + master_null_bytes;
   Field **const end_ptr= begin_ptr + colcnt;
+
+  DBUG_ASSERT(null_ptr < row_data + master_null_byte_count);
+
+  // Mask to mask out the correct bit among the null bits
+  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)
   {
     Field *const f= *field_ptr;
 
     if (bitmap_is_set(cols, field_ptr -  begin_ptr))
     {
+      if ((null_mask & 0xFF) == 0)
+      {
+        DBUG_ASSERT(null_ptr < row_data + master_null_byte_count);
+        null_mask= 1U;
+        null_bits= *null_ptr++;
+      }
+
+      DBUG_ASSERT(null_mask & 0xFF); // One of the 8 LSB should be set
+
+      /*
+        Currently, we always need to unpack the row even if it was
+        NULL.
+       */
       f->move_field_offset(offset);
-      ptr= f->unpack(f->ptr, ptr);
+      pack_ptr= f->unpack(f->ptr, pack_ptr);
       f->move_field_offset(-offset);
+
       /* Field...::unpack() cannot return 0 */
-      DBUG_ASSERT(ptr != NULL);
+      DBUG_ASSERT(pack_ptr != NULL);
+
+      if ((null_bits & null_mask) && f->maybe_null())
+        f->set_null(offset);
+      else
+        f->set_notnull(offset);
+
+      bitmap_set_bit(rw_set, field_ptr - begin_ptr);
+      null_mask <<= 1;
     }
-    else
-      bitmap_clear_bit(rw_set, field_ptr - begin_ptr);
   }
 
-  *row_end = ptr;
+  /*
+    We should now have read all the null bytes, otherwise something is
+    really wrong.
+   */
+  DBUG_ASSERT(null_ptr == row_data + master_null_byte_count);
+
+  *row_end = pack_ptr;
   if (master_reclength)
   {
     if (*field_ptr)
@@ -5562,10 +5575,6 @@
   for ( ; *field_ptr ; ++field_ptr)
   {
     uint32 const mask= NOT_NULL_FLAG | NO_DEFAULT_VALUE_FLAG;
-
-    DBUG_PRINT("debug", ("flags = 0x%x, mask = 0x%x, flags & mask = 0x%x",
-                         (*field_ptr)->flags, mask,
-                         (*field_ptr)->flags & mask));
 
     if (event_type == WRITE_ROWS_EVENT &&
         ((*field_ptr)->flags & mask) == mask)

--- 1.293/sql/sql_class.cc	2006-12-14 05:23:39 +01:00
+++ 1.294/sql/sql_class.cc	2006-12-14 05:23:39 +01:00
@@ -2527,30 +2527,110 @@
 }
 
 
-my_size_t THD::pack_row(TABLE *table, MY_BITMAP const* cols, byte *row_data, 
-                        const byte *record) const
+/*
+  Pack a record of data for a table into a format suitable for
+  transfer via the binary log.
+
+  SYNOPSIS
+    THD::pack_row()
+    table     Table describing the format of the record
+    cols      Bitmap with a set bit for each column that should be
+              stored in the row
+    row_data  Pointer to memory where row will be written
+    record    Pointer to record that should be packed. It is assumed
+              that the pointer refers to either record[0] or
+              record[1], but no such check is made since the code does
+              not rely on that.
+
+  DESCRIPTION
+
+    The format for a row in transfer with N fields is the following:
+
+    ceil(N/8) null bytes:
+        One null bit for every column *regardless of whether it can be
+        null or not*. This simplifies the decoding. Observe that the
+        number of null bits is equal to the number of set bits in the
+        'cols' bitmap. The number of null bytes is the smallest number
+        of bytes necessary to store the null bits.
+
+        Padding bits are 1.
+
+    N packets:
+        Each field is stored in packed format.
+
+
+  RETURN VALUE
+
+    The number of bytes written at 'row_data'.
+ */
+my_size_t
+THD::pack_row(TABLE *table, MY_BITMAP const* cols,
+              byte *const row_data, const byte *record) const
 {
   Field **p_field= table->field, *field;
-  int n_null_bytes= table->s->null_bytes;
-  byte *ptr;
-  uint i;
+  int const null_byte_count= (bitmap_bits_set(cols) + 7) / 8;
+  byte *pack_ptr = row_data + null_byte_count;
+  byte *null_ptr = row_data;
   my_ptrdiff_t const rec_offset= record - table->record[0];
   my_ptrdiff_t const def_offset= table->s->default_values - table->record[0];
-  memcpy(row_data, record, n_null_bytes);
-  ptr= row_data+n_null_bytes;
 
-  for (i= 0 ; (field= *p_field) ; i++, p_field++)
+  /*
+    We write the null bits and the packed records using one pass
+    through all the fields. The null bytes are written little-endian,
+    i.e., the first fields are in the first byte.
+   */
+  unsigned int null_bits= (1U << 8) - 1;
+  // Mask to mask out the correct but among the null bits
+  unsigned int null_mask= 1U;
+  for ( ; (field= *p_field) ; p_field++)
   {
-    if (bitmap_is_set(cols,i))
+    DBUG_PRINT("debug", ("null_mask=%d; null_ptr=%p; row_data=%p; null_byte_count=%d",
+                         null_mask, null_ptr, row_data, null_byte_count));
+    if (bitmap_is_set(cols, p_field - table->field))
     {
-      my_ptrdiff_t const offset=
-        field->is_null(rec_offset) ? def_offset : rec_offset;
+      my_ptrdiff_t offset;
+      if (field->is_null(rec_offset))
+      {
+        offset= def_offset;
+        null_bits |= null_mask;
+      }
+      else
+      {
+        offset= rec_offset;
+        null_bits &= ~null_mask;
+      }
+
+      null_mask <<= 1;
+      if ((null_mask & 0xFF) == 0)
+      {
+        DBUG_ASSERT(null_ptr < row_data + null_byte_count);
+        null_mask = 1U;
+        *null_ptr++ = null_bits;
+        null_bits= (1U << 8) - 1;
+      }
+
       field->move_field_offset(offset);
-      ptr= (byte*)field->pack((char *) ptr, field->ptr);
+      pack_ptr= (byte*)field->pack((char *) pack_ptr, field->ptr);
       field->move_field_offset(-offset);
     }
   }
-  return (static_cast<my_size_t>(ptr - row_data));
+
+  /*
+    Write the last (partial) byte, if there is one
+  */
+  if ((null_mask & 0xFF) > 1)
+  {
+    DBUG_ASSERT(null_ptr < row_data + null_byte_count);
+    *null_ptr++ = null_bits;
+  }
+
+  /*
+    The null pointer should now point to the first byte of the
+    packed data. If it doesn't, something is very wrong.
+  */
+  DBUG_ASSERT(null_ptr == row_data + null_byte_count);
+
+  return static_cast<my_size_t>(pack_ptr - row_data);
 }
 
 

--- 1.11/mysql-test/r/rpl_row_basic_11bugs.result	2006-12-14 05:23:39 +01:00
+++ 1.12/mysql-test/r/rpl_row_basic_11bugs.result	2006-12-14 05:23:39 +01:00
@@ -120,3 +120,102 @@
 SELECT HEX(a),b FROM t1;
 HEX(a)	b
 0	2
+DROP TABLE IF EXISTS t1;
+================ Test for BUG#22583 ================
+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;
+**** On Master **** 
+CREATE TABLE t1_myisam (k INT, a BIT(1), b BIT(9)) ENGINE=MYISAM;
+CREATE TABLE t1_innodb (k INT, a BIT(1), b BIT(9)) ENGINE=INNODB;
+CREATE TABLE t2_myisam (k INT, a BIT(1) NOT NULL, b BIT(4) NOT NULL) ENGINE=MYISAM;
+CREATE TABLE t2_innodb (k INT, a BIT(1) NOT NULL, b BIT(4) NOT NULL) ENGINE=INNODB;
+**** On Slave **** 
+ALTER TABLE t1_myisam ENGINE=INNODB;
+ALTER TABLE t1_innodb ENGINE=MYISAM;
+ALTER TABLE t2_myisam ENGINE=INNODB;
+ALTER TABLE t2_innodb ENGINE=MYISAM;
+**** On Master **** 
+INSERT INTO t1_myisam VALUES(1, b'0', 257);
+INSERT INTO t1_myisam VALUES(2, b'1', 256);
+INSERT INTO t1_innodb VALUES(1, b'0', 257);
+INSERT INTO t1_innodb VALUES(2, b'1', 256);
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+k	HEX(a)	HEX(b)
+1	0	101
+2	1	100
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+k	HEX(a)	HEX(b)
+1	0	101
+2	1	100
+INSERT INTO t2_myisam VALUES(1, b'0', 9);
+INSERT INTO t2_myisam VALUES(2, b'1', 8);
+INSERT INTO t2_innodb VALUES(1, b'0', 9);
+INSERT INTO t2_innodb VALUES(2, b'1', 8);
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+k	HEX(a)	HEX(b)
+1	0	9
+2	1	8
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+k	HEX(a)	HEX(b)
+1	0	9
+2	1	8
+**** On Slave **** 
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+k	HEX(a)	HEX(b)
+1	0	101
+2	1	100
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+k	HEX(a)	HEX(b)
+1	0	101
+2	1	100
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+k	HEX(a)	HEX(b)
+1	0	9
+2	1	8
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+k	HEX(a)	HEX(b)
+1	0	9
+2	1	8
+**** On Master **** 
+UPDATE t1_myisam SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+k	HEX(a)	HEX(b)
+1	0	101
+2	0	100
+UPDATE t1_innodb SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+k	HEX(a)	HEX(b)
+1	0	101
+2	0	100
+UPDATE t2_myisam SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+k	HEX(a)	HEX(b)
+1	0	9
+2	0	8
+UPDATE t2_innodb SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+k	HEX(a)	HEX(b)
+1	0	9
+2	0	8
+**** On Slave **** 
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+k	HEX(a)	HEX(b)
+1	0	101
+2	0	100
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+k	HEX(a)	HEX(b)
+1	0	101
+2	0	100
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+k	HEX(a)	HEX(b)
+1	0	9
+2	0	8
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+k	HEX(a)	HEX(b)
+1	0	9
+2	0	8
+**** On Master **** 
+DROP TABLE IF EXISTS t1_myisam, t1_innodb, t2_myisam, t2_innodb;

--- 1.2/mysql-test/t/rpl_row_basic_11bugs-master.opt	2006-12-14 05:23:39 +01:00
+++ 1.3/mysql-test/t/rpl_row_basic_11bugs-master.opt	2006-12-14 05:23:39 +01:00
@@ -1 +1,2 @@
---binlog_ignore_db=test_ignore
+--binlog_ignore_db=test_ignore --innodb
+

--- 1.10/mysql-test/t/rpl_row_basic_11bugs.test	2006-12-14 05:23:39 +01:00
+++ 1.11/mysql-test/t/rpl_row_basic_11bugs.test	2006-12-14 05:23:39 +01:00
@@ -114,3 +114,71 @@
 SELECT HEX(a),b FROM t1;
 sync_slave_with_master;
 SELECT HEX(a),b FROM t1;
+
+connection master;
+DROP TABLE IF EXISTS t1;
+sync_slave_with_master;
+
+# BUG#22583: RBR between MyISAM and non-MyISAM tables containing a BIT
+# field does not work
+
+--echo ================ Test for BUG#22583 ================
+--disable_query_log
+--source include/master-slave-reset.inc
+--enable_query_log
+
+--echo **** On Master **** 
+connection master;
+CREATE TABLE t1_myisam (k INT, a BIT(1), b BIT(9)) ENGINE=MYISAM;
+CREATE TABLE t1_innodb (k INT, a BIT(1), b BIT(9)) ENGINE=INNODB;
+CREATE TABLE t2_myisam (k INT, a BIT(1) NOT NULL, b BIT(4) NOT NULL) ENGINE=MYISAM;
+CREATE TABLE t2_innodb (k INT, a BIT(1) NOT NULL, b BIT(4) NOT NULL) ENGINE=INNODB;
+--echo **** On Slave **** 
+sync_slave_with_master;
+ALTER TABLE t1_myisam ENGINE=INNODB;
+ALTER TABLE t1_innodb ENGINE=MYISAM;
+ALTER TABLE t2_myisam ENGINE=INNODB;
+ALTER TABLE t2_innodb ENGINE=MYISAM;
+
+--echo **** On Master **** 
+connection master;
+INSERT INTO t1_myisam VALUES(1, b'0', 257);
+INSERT INTO t1_myisam VALUES(2, b'1', 256);
+INSERT INTO t1_innodb VALUES(1, b'0', 257);
+INSERT INTO t1_innodb VALUES(2, b'1', 256);
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+INSERT INTO t2_myisam VALUES(1, b'0', 9);
+INSERT INTO t2_myisam VALUES(2, b'1', 8);
+INSERT INTO t2_innodb VALUES(1, b'0', 9);
+INSERT INTO t2_innodb VALUES(2, b'1', 8);
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+--echo **** On Slave **** 
+sync_slave_with_master;
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+
+--echo **** On Master **** 
+connection master;
+UPDATE t1_myisam SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+UPDATE t1_innodb SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+UPDATE t2_myisam SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+UPDATE t2_innodb SET a=0 WHERE k=2;
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+--echo **** On Slave **** 
+sync_slave_with_master;
+SELECT k, HEX(a),HEX(b) FROM t1_myisam;
+SELECT k, HEX(a),HEX(b) FROM t1_innodb;
+SELECT k, HEX(a),HEX(b) FROM t2_myisam;
+SELECT k, HEX(a),HEX(b) FROM t2_innodb;
+
+--echo **** On Master **** 
+connection master;
+DROP TABLE IF EXISTS t1_myisam, t1_innodb, t2_myisam, t2_innodb;
+sync_slave_with_master;
--- New file ---
+++ mysql-test/t/rpl_row_basic_11bugs-slave.opt	06/12/14 05:23:25
--innodb

Thread
bk commit into 5.1 tree (mats:1.2308) BUG#22583Mats Kindahl14 Dec