Chuck,
The patch looks fine and I also verified that it solves the problem in my test
environment. Hence it is not only good but very desirable to push! :) Great job
locating and fixing this problem!
Btw., I noticed that on my machine I can now pass all the tests in "--valgrind"
mode (after disabling the "not_valgrind.inc" options). If you think the valgrind
failures we observed before were related to this issue, perhaps we could take a
risk and valgrind-enable all the tests again. If we do so, I suggest that we
keep this change as a separate patch.
Rafal
cbell@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 6.0 repository of cbell. When cbell 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, 2007-12-14 19:45:09-05:00, cbell@mysql_cab_desk. +11 -0
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> Patch corrects invalid restore of MyISAM table data on restore.
>
> mysql-test/r/backup.result@stripped, 2007-12-14 19:44:56-05:00, cbell@mysql_cab_desk. +2
> -1
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> New result file.
>
> mysql-test/r/backup_ddl_blocker.result@stripped, 2007-12-14 19:44:57-05:00,
> cbell@mysql_cab_desk. +5 -5
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> New result file.
>
> mysql-test/r/backup_no_data.result@stripped, 2007-12-14 19:44:58-05:00,
> cbell@mysql_cab_desk. +1 -1
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> New result file.
>
> mysql-test/r/backup_no_engine.result@stripped, 2007-12-14 19:44:58-05:00,
> cbell@mysql_cab_desk. +2 -2
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> New result file.
>
> mysql-test/r/backup_progress.result@stripped, 2007-12-14 19:44:59-05:00,
> cbell@mysql_cab_desk. +1 -1
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> New result file.
>
> mysql-test/t/backup.test@stripped, 2007-12-14 19:44:59-05:00, cbell@mysql_cab_desk. +4
> -2
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> Corrections to test to restore the use of MyISAM tables.
>
> mysql-test/t/backup_ddl_blocker.test@stripped, 2007-12-14 19:45:00-05:00,
> cbell@mysql_cab_desk. +5 -5
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> Corrections to test to restore the use of MyISAM tables.
>
> mysql-test/t/backup_no_data.test@stripped, 2007-12-14 19:45:00-05:00,
> cbell@mysql_cab_desk. +1 -1
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> Corrections to test to restore the use of MyISAM tables.
>
> mysql-test/t/backup_no_engine.test@stripped, 2007-12-14 19:45:01-05:00,
> cbell@mysql_cab_desk. +1 -1
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> Corrections to test to restore the use of MyISAM tables.
>
> mysql-test/t/backup_progress.test@stripped, 2007-12-14 19:45:02-05:00,
> cbell@mysql_cab_desk. +1 -1
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> Corrections to test to restore the use of MyISAM tables.
>
> sql/backup/be_default.cc@stripped, 2007-12-14 19:45:02-05:00, cbell@mysql_cab_desk. +5
> -1
> BUG#32954 : Online Backup: MyISAM default driver misses rows in release build
>
> Call restore_record() before unpack().
> Call ha_write_row() instead of write_row().
>
> diff -Nrup a/mysql-test/r/backup.result b/mysql-test/r/backup.result
> --- a/mysql-test/r/backup.result 2007-12-13 11:30:21 -05:00
> +++ b/mysql-test/r/backup.result 2007-12-14 19:44:56 -05:00
> @@ -48,7 +48,8 @@ DROP DATABASE db1;
> DROP DATABASE db2;
> USE mysql;
> RESTORE FROM 'test.ba';
> -backup_id #
> +backup_id
> +#
> SHOW CREATE DATABASE db1;
> Database Create Database
> db1 CREATE DATABASE `db1` /*!40100 DEFAULT CHARACTER SET latin1 */
> diff -Nrup a/mysql-test/r/backup_ddl_blocker.result
> b/mysql-test/r/backup_ddl_blocker.result
> --- a/mysql-test/r/backup_ddl_blocker.result 2007-12-13 09:11:55 -05:00
> +++ b/mysql-test/r/backup_ddl_blocker.result 2007-12-14 19:44:57 -05:00
> @@ -631,7 +631,7 @@ CREATE DATABASE bup_ddl_blocker_2;
> CREATE DATABASE bup_ddl_blocker_4;
> con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
> con1: Loading data
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("01 Some data to test");
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("02 Some data to test");
> @@ -735,7 +735,7 @@ CREATE DATABASE bup_ddl_blocker_2;
> CREATE DATABASE bup_ddl_blocker_4;
> con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
> con1: Loading data
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("01 Some data to test");
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("02 Some data to test");
> @@ -826,7 +826,7 @@ CREATE DATABASE bup_ddl_blocker_2 CHARAC
> CREATE DATABASE bup_ddl_blocker_4 CHARACTER SET latin1;
> con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
> con1: Loading data
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("01 Some data to test");
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("02 Some data to test");
> @@ -959,7 +959,7 @@ CREATE DATABASE bup_ddl_blocker_2 CHARAC
> CREATE DATABASE bup_ddl_blocker_4 CHARACTER SET latin1;
> con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
> con1: Loading data
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("01 Some data to test");
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("02 Some data to test");
> @@ -1062,7 +1062,7 @@ DROP TABLE IF EXISTS test.t2;
> DROP TABLE IF EXISTS bup_ddl_blocker.t1, bup_ddl_blocker.t3;
> con1: Creating tables
> CREATE TABLE bup_ddl_blocker.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE test.t2 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE test.t2 (col_a CHAR(40)) ENGINE=MYISAM;
> CREATE TABLE bup_ddl_blocker.t3 (col_a CHAR(40)) ENGINE=MEMORY;
> con1: Loading data
> INSERT INTO bup_ddl_blocker.t1 VALUES ("01 Some data to test");
> diff -Nrup a/mysql-test/r/backup_no_data.result b/mysql-test/r/backup_no_data.result
> --- a/mysql-test/r/backup_no_data.result 2007-12-03 15:28:03 -05:00
> +++ b/mysql-test/r/backup_no_data.result 2007-12-14 19:44:58 -05:00
> @@ -51,7 +51,7 @@ Note 1051 Unknown table 't1'
> CREATE TABLE t1 (
> `dir_code` char(4),
> `building` char(6)
> -) ENGINE=MEMORY DEFAULT CHARSET=latin1;
> +) ENGINE=MYISAM DEFAULT CHARSET=latin1;
> USE empty_db;
> DROP VIEW IF EXISTS v1;
> CREATE VIEW v1 AS SELECT * FROM test.t1;
> diff -Nrup a/mysql-test/r/backup_no_engine.result
> b/mysql-test/r/backup_no_engine.result
> --- a/mysql-test/r/backup_no_engine.result 2007-12-03 15:28:04 -05:00
> +++ b/mysql-test/r/backup_no_engine.result 2007-12-14 19:44:58 -05:00
> @@ -1,7 +1,7 @@
> DROP DATABASE IF EXISTS db;
> CREATE DATABASE db;
> CREATE TABLE db.t1 (a int, b char(32))
> -ENGINE=MEMORY;
> +ENGINE=MYISAM;
> BACKUP DATABASE db TO "db.backup";
> backup_id
> #
> @@ -18,5 +18,5 @@ Table Create Table
> t1 CREATE TABLE `t1` (
> `a` int(11) DEFAULT NULL,
> `b` char(32) DEFAULT NULL
> -) ENGINE=MEMORY DEFAULT CHARSET=latin1
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> DROP DATABASE db;
> diff -Nrup a/mysql-test/r/backup_progress.result
> b/mysql-test/r/backup_progress.result
> --- a/mysql-test/r/backup_progress.result 2007-12-04 12:38:02 -05:00
> +++ b/mysql-test/r/backup_progress.result 2007-12-14 19:44:59 -05:00
> @@ -3,7 +3,7 @@ DROP TABLE IF EXISTS backup_progress.t1_
> SET GLOBAL debug="d,backup_debug:d,backup";
> CREATE DATABASE backup_progress;
> con1: Create table and new users.
> -CREATE TABLE backup_progress.t1 (a char(30)) ENGINE=MEMORY;
> +CREATE TABLE backup_progress.t1 (a char(30)) ENGINE=MYISAM;
> CREATE TABLE backup_progress.t2 (a char(30)) ENGINE=INNODB;
> CREATE TABLE backup_progress.t3 (a char(30)) ENGINE=MEMORY;
> CREATE TABLE backup_progress.t1_res (id INT);
> diff -Nrup a/mysql-test/t/backup.test b/mysql-test/t/backup.test
> --- a/mysql-test/t/backup.test 2007-12-13 11:30:22 -05:00
> +++ b/mysql-test/t/backup.test 2007-12-14 19:44:59 -05:00
> @@ -103,8 +103,10 @@ DROP DATABASE db2;
>
> USE mysql;
>
> ---replace_column 1 # 2 # 3 # 4 # 10 # 11 # 12 #
> ---query_vertical RESTORE FROM 'test.ba'
> +--replace_column 1 #
> +RESTORE FROM 'test.ba';
> +
> +--remove_file $MYSQLTEST_VARDIR/master-data/test.ba
>
> SHOW CREATE DATABASE db1;
> SHOW CREATE DATABASE db2;
> diff -Nrup a/mysql-test/t/backup_ddl_blocker.test
> b/mysql-test/t/backup_ddl_blocker.test
> --- a/mysql-test/t/backup_ddl_blocker.test 2007-12-13 09:11:55 -05:00
> +++ b/mysql-test/t/backup_ddl_blocker.test 2007-12-14 19:45:00 -05:00
> @@ -1060,7 +1060,7 @@ CREATE DATABASE bup_ddl_blocker_4;
> # Create transaction tables and load them with data.
> --echo con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
>
> --echo con1: Loading data
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("01 Some data to test");
> @@ -1245,7 +1245,7 @@ CREATE DATABASE bup_ddl_blocker_4;
> # Create transaction tables and load them with data.
> --echo con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
>
> --echo con1: Loading data
> INSERT INTO bup_ddl_blocker_2.t1 VALUES ("01 Some data to test");
> @@ -1419,7 +1419,7 @@ CREATE DATABASE bup_ddl_blocker_4 CHARAC
> # Create transaction tables and load them with data.
> --echo con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
>
> --echo con1: Loading data
>
> @@ -1618,7 +1618,7 @@ CREATE DATABASE bup_ddl_blocker_4 CHARAC
> # Create transaction tables and load them with data.
> --echo con1: Creating tables
> CREATE TABLE bup_ddl_blocker_2.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE bup_ddl_blocker_4.t1 (col_a CHAR(40)) ENGINE=MYISAM;
>
> --echo con1: Loading data
>
> @@ -1815,7 +1815,7 @@ DROP TABLE IF EXISTS bup_ddl_blocker.t1,
> # Create transaction tables and load them with data.
> --echo con1: Creating tables
> CREATE TABLE bup_ddl_blocker.t1 (col_a CHAR(40)) ENGINE=INNODB;
> -CREATE TABLE test.t2 (col_a CHAR(40)) ENGINE=MEMORY;
> +CREATE TABLE test.t2 (col_a CHAR(40)) ENGINE=MYISAM;
> CREATE TABLE bup_ddl_blocker.t3 (col_a CHAR(40)) ENGINE=MEMORY;
>
> --echo con1: Loading data
> diff -Nrup a/mysql-test/t/backup_no_data.test b/mysql-test/t/backup_no_data.test
> --- a/mysql-test/t/backup_no_data.test 2007-12-03 15:28:08 -05:00
> +++ b/mysql-test/t/backup_no_data.test 2007-12-14 19:45:00 -05:00
> @@ -52,7 +52,7 @@ DROP TABLE IF EXISTS t1;
> CREATE TABLE t1 (
> `dir_code` char(4),
> `building` char(6)
> -) ENGINE=MEMORY DEFAULT CHARSET=latin1;
> +) ENGINE=MYISAM DEFAULT CHARSET=latin1;
>
> USE empty_db;
>
> diff -Nrup a/mysql-test/t/backup_no_engine.test b/mysql-test/t/backup_no_engine.test
> --- a/mysql-test/t/backup_no_engine.test 2007-12-03 15:28:09 -05:00
> +++ b/mysql-test/t/backup_no_engine.test 2007-12-14 19:45:01 -05:00
> @@ -13,7 +13,7 @@ DROP DATABASE IF EXISTS db;
>
> CREATE DATABASE db;
> CREATE TABLE db.t1 (a int, b char(32))
> -ENGINE=MEMORY;
> +ENGINE=MYISAM;
>
> # copy description of a table using non-existent storage engine
> --copy_file $table_def $MYSQLTEST_VARDIR/master-data/db/t2.frm
> diff -Nrup a/mysql-test/t/backup_progress.test b/mysql-test/t/backup_progress.test
> --- a/mysql-test/t/backup_progress.test 2007-12-06 16:42:58 -05:00
> +++ b/mysql-test/t/backup_progress.test 2007-12-14 19:45:02 -05:00
> @@ -30,7 +30,7 @@ CREATE DATABASE backup_progress;
>
> --echo con1: Create table and new users.
>
> -CREATE TABLE backup_progress.t1 (a char(30)) ENGINE=MEMORY;
> +CREATE TABLE backup_progress.t1 (a char(30)) ENGINE=MYISAM;
> CREATE TABLE backup_progress.t2 (a char(30)) ENGINE=INNODB;
> CREATE TABLE backup_progress.t3 (a char(30)) ENGINE=MEMORY;
> CREATE TABLE backup_progress.t1_res (id INT);
> diff -Nrup a/sql/backup/be_default.cc b/sql/backup/be_default.cc
> --- a/sql/backup/be_default.cc 2007-12-13 09:08:41 -05:00
> +++ b/sql/backup/be_default.cc 2007-12-14 19:45:02 -05:00
> @@ -671,6 +671,8 @@ uint Restore::unpack(byte *packed_row)
> /* Potential buffer on the stack for the bitmap */
> uint32 bitbuf[BITMAP_STACKBUF_SIZE/sizeof(uint32)];
> uint n_fields= cur_table->s->fields;
> + /* Restore a default record -- MyISAM needs this to work properly. */
> + restore_record(cur_table, s->default_values);
> 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);
> @@ -887,7 +889,9 @@ result_t Restore::send_data(Buffer &buf)
> }
> if (write_row)
> {
> - last_write_res = hdl->write_row(cur_table->record[0]);
> + last_write_res = hdl->ha_write_row(cur_table->record[0]);
> + DBUG_PRINT("backup_default_write", ("%d", last_write_res));
> +
> /*
> Free the blob pointers used.
> */
>
>
