Hi Chuck,
The patch is ok to push. However I miss an explanation what caused the problem
described in the bug report (assertion failure) and how this patch fixes it.
Missing such explanation makes me less sure that we really fixed the original
problem. However, the change of the patch is definitely needed and testing shows
that the problem disappears.
Apply my suggestion before pushing if you like. No need to review it again.
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, 2008-05-07 12:23:44-04:00, cbell@mysql_cab_desk. +3 -0
> BUG#35249 : Mysql server crash for delete operation followed by backup in memory
> SE
>
> Backup fails when deleted rows encountered. Default drivers do not
> process rows marked as deleted properly. Patch allows default and
> consistent snapshot drivers to skip deleted rows on backup.
>
> mysql-test/r/backup.result@stripped, 2008-05-07 12:23:39-04:00, cbell@mysql_cab_desk.
> +35 -0
> BUG#35249 : Mysql server crash for delete operation followed by backup in memory
> SE
>
> New result file.
>
> mysql-test/t/backup.test@stripped, 2008-05-07 12:23:39-04:00, cbell@mysql_cab_desk. +32
> -0
> BUG#35249 : Mysql server crash for delete operation followed by backup in memory
> SE
>
> New test to backup when rows are deleted.
>
> sql/backup/be_default.cc@stripped, 2008-05-07 12:23:40-04:00, cbell@mysql_cab_desk. +5
> -0
> BUG#35249 : Mysql server crash for delete operation followed by backup in memory
> SE
>
> Added code to skip deleted rows.
>
> diff -Nrup a/mysql-test/r/backup.result b/mysql-test/r/backup.result
> --- a/mysql-test/r/backup.result 2008-03-31 03:16:54 -04:00
> +++ b/mysql-test/r/backup.result 2008-05-07 12:23:39 -04:00
> @@ -523,4 +523,39 @@ COUNT(*)
> SELECT COUNT(*) FROM bup_default.wide;
> COUNT(*)
> 1
> +create the database
> +CREATE DATABASE bup_delete;
> +create a table and populate it
> +CREATE TABLE bup_delete.me(id int, ccode varchar(20)) ENGINE=MEMORY;
> +INSERT INTO bup_delete.me VALUES (1,'aa'),(2,'bb'),(3,'cc'),(4,'dd');
> +show the data
> +SELECT * FROM bup_delete.me;
> +id ccode
> +1 aa
> +2 bb
> +3 cc
> +4 dd
> +delete a row
> +DELETE FROM bup_delete.me WHERE ccode='cc';
> +do backup
> +BACKUP DATABASE bup_delete TO 'bup_delete.bak';
> +backup_id
> +#
> +show the data
> +SELECT * FROM bup_delete.me;
> +id ccode
> +1 aa
> +2 bb
> +4 dd
> +do restore
> +RESTORE FROM 'bup_delete.bak';
> +backup_id
> +#
> +show the data
> +SELECT * FROM bup_delete.me;
> +id ccode
> +1 aa
> +2 bb
> +4 dd
> +DROP DATABASE IF EXISTS bup_delete;
> DROP DATABASE IF EXISTS bup_default;
> diff -Nrup a/mysql-test/t/backup.test b/mysql-test/t/backup.test
> --- a/mysql-test/t/backup.test 2008-03-31 03:16:54 -04:00
> +++ b/mysql-test/t/backup.test 2008-05-07 12:23:39 -04:00
> @@ -391,11 +391,43 @@ SELECT * FROM bup_default.t2;
> SELECT COUNT(*) FROM bup_default.t1_blob;
> SELECT COUNT(*) FROM bup_default.wide;
>
> +#
> +# BUG#35249 : Test backup when rows are deleted
> +#
> +
> +--echo create the database
> +CREATE DATABASE bup_delete;
> +
> +--echo create a table and populate it
> +CREATE TABLE bup_delete.me(id int, ccode varchar(20)) ENGINE=MEMORY;
> +INSERT INTO bup_delete.me VALUES (1,'aa'),(2,'bb'),(3,'cc'),(4,'dd');
> +
> +--echo show the data
> +SELECT * FROM bup_delete.me;
> +
> +--echo delete a row
> +DELETE FROM bup_delete.me WHERE ccode='cc';
> +
> +--echo do backup
> +--replace_column 1 #
> +BACKUP DATABASE bup_delete TO 'bup_delete.bak';
> +
> +--echo show the data
> +SELECT * FROM bup_delete.me;
> +
> +--echo do restore
> +--replace_column 1 #
> +RESTORE FROM 'bup_delete.bak';
> +
> +--echo show the data
> +SELECT * FROM bup_delete.me;
>
> --disable_warnings
> +DROP DATABASE IF EXISTS bup_delete;
> DROP DATABASE IF EXISTS bup_default;
> --enable_warnings
>
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_delete.bak
> --remove_file $MYSQLTEST_VARDIR/master-data/bup_default.bak
> --error 0,1
> --remove_file $MYSQLTEST_VARDIR/master-data/test.ba
> diff -Nrup a/sql/backup/be_default.cc b/sql/backup/be_default.cc
> --- a/sql/backup/be_default.cc 2008-03-04 11:08:40 -05:00
> +++ b/sql/backup/be_default.cc 2008-05-07 12:23:40 -04:00
> @@ -363,6 +363,11 @@ result_t Backup::get_data(Buffer &buf)
> cur_blob= 0;
> cur_table->use_all_columns();
> last_read_res = hdl->rnd_next(cur_table->record[0]);
> + /*
> + Skip all records marked as deleted.
> + */
> + while (last_read_res == HA_ERR_RECORD_DELETED)
> + last_read_res = hdl->rnd_next(cur_table->record[0]);
Perhaps use
do {
last_read_res= hdl->rnd_next(cur_table->record[0]);
} while (last_read_res == HA_ERR_RECORD_DELETED)
to avoid code repetition. (Note invalid spacing around '=' in your code).
> DBUG_EXECUTE_IF("SLEEP_DRIVER", sleep(4););
> /*
> If we are end of file, stop the read process and signal the
>
>
