From: Jorgen Loland Date: September 9 2008 8:20am Subject: bzr push into mysql-6.0-backup branch (jorgen.loland:2696) Bug#37212 List-Archive: http://lists.mysql.com/commits/53573 X-Bug: 37212 Message-Id: <20080909082033.EA5BE3B8024@atum06.norway.sun.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 === 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,