List:Commits« Previous MessageNext Message »
From:Jorgen Loland Date:September 9 2008 8:19am
Subject:bzr commit into mysql-6.0-backup branch (jorgen.loland:2696) Bug#37212
View as plain text  
#At file:///localhome/jl208045/mysql/mysql-6.0-backup-37212/

 2696 Jorgen Loland	2008-09-09
      Bug#37212 - Restore crashes if table has longblob of size 1MB
            
      Backup/restore assumed that rpl_record.cc (un)pack_row did not 
      (un)pack blob fields. Instead, b/r handles blob fields by using
      the field.h get/set_ptr() methods. However, (un)pack_row did 
      not ignore blob fields, resulting in crash because backup/restore
      buffer was not big enough to handle 1MB blobs.
            
      rpl_record.cc (un)pack_row now takes an option whether to 
      operate on blobs or not.
added:
  mysql-test/r/backup_blob.result
  mysql-test/t/backup_blob.test
modified:
  sql/backup/be_default.cc
  sql/backup/stream_v1.c
  sql/field.h
  sql/log_event.h
  sql/log_event_old.h
  sql/rpl_record.cc
  sql/rpl_record.h
  sql/sql_class.cc

per-file comments:
  mysql-test/r/backup_blob.result
    Restults from new blob test file
  mysql-test/t/backup_blob.test
    Test for backup/restore of blobs
  sql/backup/be_default.cc
    Modified calls to (un)pack_row to handle new method signature, and call set_notnull after calling set_ptr when restoring blob fields.
  sql/field.h
    Added documentation for set_ptr
  sql/log_event.h
    Modified call to unpack_row to handle new method signature.
  sql/log_event_old.h
    Modified call to unpack_row to handle new method signature.
  sql/rpl_record.cc
    Added option to (un)pack_row: whether or not to ignore blob fields during pack and unpack.
  sql/rpl_record.h
    Added option to (un)pack_row: whether or not to ignore blob fields during pack and unpack.
  sql/sql_class.cc
    Modified calls to rpl_record pack_row to handle new method signature.
=== added file 'mysql-test/r/backup_blob.result'
--- a/mysql-test/r/backup_blob.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/backup_blob.result	2008-09-09 08:19:21 +0000
@@ -0,0 +1,52 @@
+DROP DATABASE IF EXISTS mysqltest;
+
+Test blobs, InnoDB
+
+CREATE DATABASE mysqltest;
+USE mysqltest;
+CREATE TABLE t1 (id int, txt longblob) ENGINE=innodb;
+INSERT INTO t1 VALUES(1, "short text");
+INSERT INTO t1 VALUES(2, "a little longer, but still short text");
+CREATE TABLE t2 (id int, txt longblob) ENGINE=innodb;
+INSERT INTO t2 VALUES(1, REPEAT('x',1000));
+INSERT INTO t2 VALUES(2, REPEAT('y',10000));
+INSERT INTO t2 VALUES(3, REPEAT('z',1048576));
+CREATE TABLE t3 (id int, txt longblob, txt2 longblob, txt3 longblob) ENGINE=innodb;
+INSERT INTO t3 VALUES(1, REPEAT('x',1048576), REPEAT('y',1048576), REPEAT('z',1048576));
+
+Check tables before backup
+SELECT * FROM t1 ORDER BY id;
+id	txt
+1	short text
+2	a little longer, but still short text
+CHECKSUM TABLE t2;
+Table	Checksum
+mysqltest.t2	1646886041
+CHECKSUM TABLE t3;
+Table	Checksum
+mysqltest.t3	2938103984
+
+Backup, drop and restore database
+BACKUP DATABASE mysqltest TO 'blob.bak';
+backup_id
+#
+DROP DATABASE mysqltest;
+RESTORE FROM 'blob.bak';
+backup_id
+#
+
+Check tables after restore
+SELECT * FROM t1 ORDER BY id;
+id	txt
+1	short text
+2	a little longer, but still short text
+CHECKSUM TABLE t2;
+Table	Checksum
+mysqltest.t2	1646886041
+CHECKSUM TABLE t3;
+Table	Checksum
+mysqltest.t3	2938103984
+
+Cleanup 
+
+DROP DATABASE mysqltest;

=== added file 'mysql-test/t/backup_blob.test'
--- a/mysql-test/t/backup_blob.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/backup_blob.test	2008-09-09 08:19:21 +0000
@@ -0,0 +1,64 @@
+###########################################################################
+# Purpose: Test backup/restore of blobs
+###########################################################################
+
+# This is currently only a regression test for bug#37212. When the
+# test is moved to the backup suite, at least one storage engine for
+# each driver should be tested.
+
+--source include/not_embedded.inc
+--source include/have_innodb.inc
+
+--disable_warnings
+DROP DATABASE IF EXISTS mysqltest;
+--enable_warnings
+
+--echo
+--echo Test blobs, InnoDB
+--echo
+
+CREATE DATABASE mysqltest;
+USE mysqltest;
+
+CREATE TABLE t1 (id int, txt longblob) ENGINE=innodb;
+INSERT INTO t1 VALUES(1, "short text");
+INSERT INTO t1 VALUES(2, "a little longer, but still short text");
+
+CREATE TABLE t2 (id int, txt longblob) ENGINE=innodb;
+INSERT INTO t2 VALUES(1, REPEAT('x',1000));
+INSERT INTO t2 VALUES(2, REPEAT('y',10000));
+INSERT INTO t2 VALUES(3, REPEAT('z',1048576));
+
+CREATE TABLE t3 (id int, txt longblob, txt2 longblob, txt3 longblob) ENGINE=innodb;
+INSERT INTO t3 VALUES(1, REPEAT('x',1048576), REPEAT('y',1048576), REPEAT('z',1048576));
+
+--echo
+--echo Check tables before backup
+SELECT * FROM t1 ORDER BY id; 
+CHECKSUM TABLE t2;
+CHECKSUM TABLE t3;
+
+--echo
+--echo Backup, drop and restore database
+
+--replace_column 1 #
+BACKUP DATABASE mysqltest TO 'blob.bak';
+
+DROP DATABASE mysqltest;
+--replace_column 1 #
+RESTORE FROM 'blob.bak';
+
+--echo
+--echo Check tables after restore
+
+SELECT * FROM t1 ORDER BY id; 
+CHECKSUM TABLE t2;
+CHECKSUM TABLE t3;
+
+--echo
+--echo Cleanup 
+--echo
+
+DROP DATABASE mysqltest;
+
+--remove_file $MYSQLTEST_VARDIR/master-data/blob.bak

=== modified file 'sql/backup/be_default.cc'
--- a/sql/backup/be_default.cc	2008-08-18 08:25:56 +0000
+++ b/sql/backup/be_default.cc	2008-09-09 08:19:21 +0000
@@ -275,7 +275,7 @@ uint Backup::pack(byte *rcd, byte *packe
     my_bool use_bitbuf= n_fields <= sizeof(bitbuf) * 8;
     error= bitmap_init(&cols, use_bitbuf ? bitbuf : NULL, (n_fields + 7) & ~7UL, FALSE);
     bitmap_set_all(&cols);
-    size= pack_row(cur_table, &cols, packed_row, rcd);
+    size= pack_row(cur_table, &cols, packed_row, rcd, FALSE);
     if (!use_bitbuf)
       bitmap_free(&cols);
   }
@@ -680,7 +680,7 @@ uint Restore::unpack(byte *packed_row)
     error= bitmap_init(&cols, use_bitbuf ? bitbuf : NULL, (n_fields + 7) & ~7UL, FALSE);
     bitmap_set_all(&cols);
     ulong length;
-    error= unpack_row(NULL, cur_table, n_fields, packed_row, &cols, &cur_row_end, &length);
+    error= unpack_row(NULL, cur_table, n_fields, packed_row, &cols, &cur_row_end, &length, FALSE);
     if (!use_bitbuf)
       bitmap_free(&cols);
   }
@@ -906,6 +906,7 @@ result_t Restore::send_data(Buffer &buf)
       memcpy(blob_ptrs[blob_ptr_index], (byte *)buf.data + META_SIZE, size);
       ((Field_blob*) cur_table->field[*cur_blob])->set_ptr(size, 
         (uchar *)blob_ptrs[blob_ptr_index]);
+      cur_table->field[*cur_blob]->set_notnull();
       blob_ptr_index++;
       mode= CHECK_BLOBS;
       DBUG_RETURN(PROCESSING);
@@ -944,6 +945,7 @@ result_t Restore::send_data(Buffer &buf)
       ptr= (byte *)blob_buffer.get_base_ptr();
       ((Field_blob*) cur_table->field[*cur_blob])->set_ptr(max_blob_size, 
         (uchar *)ptr);
+      cur_table->field[*cur_blob]->set_notnull();
       blob_ptr_index++;
       mode= CHECK_BLOBS;
       DBUG_RETURN(PROCESSING);

=== modified file 'sql/backup/stream_v1.c'
--- a/sql/backup/stream_v1.c	2008-08-13 12:41:33 +0000
+++ b/sql/backup/stream_v1.c	2008-09-09 08:19:21 +0000
@@ -1811,9 +1811,11 @@ int bstream_rd_data_chunk(backup_stream 
       {
         memmove(buf->begin, chunk->data.begin, howmuch);
         envelope= buf;
-        chunk->data= *buf;
       }
 
+      // update chunk->data to point to the new buffer
+      chunk->data= *buf;
+
       /* update to_read blob to indicate free space left */
       to_read.begin= buf->begin + howmuch;
       to_read.end= buf->end;

=== modified file 'sql/field.h'
--- a/sql/field.h	2008-08-28 11:17:29 +0000
+++ b/sql/field.h	2008-09-09 08:19:21 +0000
@@ -1759,6 +1759,18 @@ public:
     {
       memcpy_fixed((uchar*) str,ptr+packlength+row_offset,sizeof(char*));
     }
+
+  /**
+     Copy value in data to the field ptr. 
+
+     NOTE
+     If the null_bit for this field is set, @c
+     set_notnull(my_ptrdiff_t) needs to be called for set_ptr to have
+     any effect when inserting a record.
+
+     @param length  Length of data
+     @param data    The value of the field
+  */
   inline void set_ptr(uchar *length, uchar *data)
     {
       memcpy(ptr,length,packlength);

=== modified file 'sql/log_event.h'
--- a/sql/log_event.h	2008-08-28 09:59:54 +0000
+++ b/sql/log_event.h	2008-09-09 08:19:21 +0000
@@ -3545,7 +3545,7 @@ protected:
     DBUG_ASSERT(m_table);
     ASSERT_OR_RETURN_ERROR(m_curr_row < m_rows_end, HA_ERR_CORRUPT_EVENT);
     int const result= ::unpack_row(rli, m_table, m_width, m_curr_row, cols,
-                                   &m_curr_row_end, &m_master_reclength);
+                                   &m_curr_row_end, &m_master_reclength, TRUE);
     if (m_curr_row_end > m_rows_end)
       my_error(ER_SLAVE_CORRUPT_EVENT, MYF(0));
     ASSERT_OR_RETURN_ERROR(m_curr_row_end <= m_rows_end, HA_ERR_CORRUPT_EVENT);

=== modified file 'sql/log_event_old.h'
--- a/sql/log_event_old.h	2008-05-29 15:44:11 +0000
+++ b/sql/log_event_old.h	2008-09-09 08:19:21 +0000
@@ -204,7 +204,7 @@ protected:
     DBUG_ASSERT(m_table);
     ASSERT_OR_RETURN_ERROR(m_curr_row < m_rows_end, HA_ERR_CORRUPT_EVENT);
     int const result= ::unpack_row(rli, m_table, m_width, m_curr_row, &m_cols,
-                                   &m_curr_row_end, &m_master_reclength);
+                                   &m_curr_row_end, &m_master_reclength, TRUE);
     ASSERT_OR_RETURN_ERROR(m_curr_row_end <= m_rows_end, HA_ERR_CORRUPT_EVENT);
     return result;
   }

=== modified file 'sql/rpl_record.cc'
--- a/sql/rpl_record.cc	2008-03-27 18:40:00 +0000
+++ b/sql/rpl_record.cc	2008-09-09 08:19:21 +0000
@@ -51,12 +51,19 @@
                    record[0] or @c record[1], but no such check is
                    made since the code does not rely on that.
 
+   @param pack_blobs Whether or not blob fields are packed. If true,
+                     row_data has to be big enough to store all blobs
+                     in this record. If false, the blobs are not
+                     stored in row_data but should instead be fetched
+                     by calling @c get_ptr
+
    @return The number of bytes written at @c row_data.
  */
 #if !defined(MYSQL_CLIENT)
 size_t
 pack_row(TABLE *table, MY_BITMAP const* cols,
-         uchar *row_data, const uchar *record)
+         uchar *row_data, const uchar *record,
+         bool pack_blobs)
 {
   Field **p_field= table->field, *field;
   int const null_byte_count= (bitmap_bits_set(cols) + 7) / 8;
@@ -92,6 +99,11 @@ pack_row(TABLE *table, MY_BITMAP const* 
         offset= def_offset;
         null_bits |= null_mask;
       }
+      else if (!pack_blobs && field->unireg_check==Field::BLOB_FIELD)
+      {
+        // The field is not empty. Fetch by calling get_ptr()
+        null_bits &= ~null_mask;
+      }
       else
       {
         offset= rec_offset;
@@ -179,6 +191,10 @@ pack_row(TABLE *table, MY_BITMAP const* 
                   Pointer to variable that will be set to the length of the
                   record on the master side
 
+   @param unpack_blobs Whether or not blob fields are unpacked. If
+                       false, the blobs must be added by calling
+                       @c set_ptr() and @c set_notnull().
+
    @retval 0 No error
 
    @retval ER_NO_DEFAULT_FOR_FIELD
@@ -191,7 +207,8 @@ int
 unpack_row(Relay_log_info const *rli,
            TABLE *table, uint const colcnt,
            uchar const *const row_data, MY_BITMAP const *cols,
-           uchar const **const row_end, ulong *const master_reclength)
+           uchar const **const row_end, ulong *const master_reclength,
+           bool unpack_blobs)
 {
   DBUG_ENTER("unpack_row");
   DBUG_ASSERT(row_data);
@@ -255,6 +272,11 @@ unpack_row(Relay_log_info const *rli,
 
         f->set_null();
       }
+      else if (!unpack_blobs && f->unireg_check==Field::BLOB_FIELD)
+      {
+        DBUG_PRINT("debug", ("Not unpacking BLOB; field_ptr: %p", field_ptr));
+        f->set_null();
+      }
       else
       {
         f->set_notnull();

=== modified file 'sql/rpl_record.h'
--- a/sql/rpl_record.h	2008-03-25 15:10:50 +0000
+++ b/sql/rpl_record.h	2008-09-09 08:19:21 +0000
@@ -20,14 +20,16 @@
 
 #if !defined(MYSQL_CLIENT)
 size_t pack_row(TABLE* table, MY_BITMAP const* cols,
-                uchar *row_data, const uchar *data);
+                uchar *row_data, const uchar *data,
+                bool pack_blobs);
 #endif
 
 #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
 int unpack_row(Relay_log_info const *rli,
                TABLE *table, uint const colcnt,
                uchar const *const row_data, MY_BITMAP const *cols,
-               uchar const **const row_end, ulong *const master_reclength);
+               uchar const **const row_end, ulong *const master_reclength,
+               bool unpack_blobs);
 
 // Fill table's record[0] with default values.
 int prepare_record(TABLE *const, const MY_BITMAP *cols, uint width, const bool);

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2008-09-04 13:46:04 +0000
+++ b/sql/sql_class.cc	2008-09-09 08:19:21 +0000
@@ -3428,7 +3428,7 @@ int THD::binlog_write_row(TABLE* table, 
 
   uchar *row_data= memory.slot(0);
 
-  size_t const len= pack_row(table, table->write_set, row_data, record);
+  size_t const len= pack_row(table, table->write_set, row_data, record, TRUE);
 
   Rows_log_event* const ev=
     binlog_prepare_pending_rows_event(table, server_id, len, is_trans,
@@ -3457,9 +3457,9 @@ int THD::binlog_update_row(TABLE* table,
   uchar *after_row= row_data.slot(1);
 
   size_t const before_size= pack_row(table, table->read_set, before_row,
-                                        before_record);
+                                     before_record, TRUE);
   size_t const after_size= pack_row(table, table->write_set, after_row,
-                                       after_record);
+                                    after_record, TRUE);
 
   /*
     Don't print debug messages when running valgrind since they can
@@ -3501,7 +3501,7 @@ int THD::binlog_delete_row(TABLE* table,
   uchar *row_data= memory.slot(0);
 
   DBUG_DUMP("table->read_set", (uchar*) table->read_set->bitmap, (table->s->fields + 7) / 8);
-  size_t const len= pack_row(table, table->read_set, row_data, record);
+  size_t const len= pack_row(table, table->read_set, row_data, record, TRUE);
 
   Rows_log_event* const ev=
     binlog_prepare_pending_rows_event(table, server_id, len, is_trans,

Thread
bzr commit into mysql-6.0-backup branch (jorgen.loland:2696) Bug#37212Jorgen Loland9 Sep