Re: bzr commit into mysql-6.0-backup branch (ogrovlen:2627) Bug#34210

[Jørgen Løland]

The patch looks good. I verified that the test case fails without the 
patch and passes with the patch.


-- 
Jørgen Løland

Oystein Grovlen wrote:
> #At file:///home/og136792/mysql/shared/mysql-6.0-backup-1/
> 
>  2627 Oystein Grovlen	2008-06-04
>       Bug#34210: Perform commit at end of restore.
>       (Restore should be a transaction by itself.)
> added:
>   mysql-test/r/backup_commit_restore.result
>   mysql-test/t/backup_commit_restore.test
> modified:
>   sql/backup/data_backup.cc
> 
> per-file messages:
>   mysql-test/r/backup_commit_restore.result
>     Result log for new test.
>   mysql-test/t/backup_commit_restore.test
>     Test that restore is automatically committed (i.e., that it is not rolled back.) 
> Tests added for different backup drivers/storage engines. Tests run both with and without
> autocommit.
>   sql/backup/data_backup.cc
>     Perform commit at end of restore.
>     (Restore should be a transaction by itself.)
> === added file 'mysql-test/r/backup_commit_restore.result'
> 
> === added file 'mysql-test/r/backup_commit_restore.result'
> --- a/mysql-test/r/backup_commit_restore.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/r/backup_commit_restore.result	2008-06-04 12:13:16 +0000
> @@ -0,0 +1,331 @@
> +CREATE DATABASE commit_test;
> +USE commit_test;
> +SET @@autocommit=0;
> +CREATE TABLE t1 (s1 CHAR(2)) ENGINE=innodb;
> +INSERT INTO t1 VALUES ('a1');
> +BACKUP DATABASE commit_test TO '81';
> +backup_id
> +#
> +RESTORE FROM '81';
> +backup_id
> +#
> +SELECT * FROM t1;
> +s1
> +a1
> +ROLLBACK;
> +SELECT * FROM t1;
> +s1
> +a1
> +COMMIT;
> +SET @@autocommit=0;
> +CREATE TABLE t2 (s1 CHAR(2)) ENGINE=Memory;
> +INSERT INTO t1 VALUES ('a2');
> +INSERT INTO t2 VALUES ('a2');
> +BACKUP DATABASE commit_test TO '82';
> +backup_id
> +#
> +RESTORE FROM '82';
> +backup_id
> +#
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +SELECT * FROM t2;
> +s1
> +a2
> +ROLLBACK;
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +SELECT * FROM t2;
> +s1
> +a2
> +COMMIT;
> +SET @@autocommit=0;
> +CREATE TABLE t3 (s1 CHAR(2));
> +INSERT INTO t1 VALUES ('a3');
> +INSERT INTO t2 VALUES ('a3');
> +INSERT INTO t3 VALUES ('a3');
> +BACKUP DATABASE commit_test TO '83';
> +backup_id
> +#
> +RESTORE FROM '83';
> +backup_id
> +#
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +SELECT * FROM t3;
> +s1
> +a3
> +ROLLBACK;
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +SELECT * FROM t3;
> +s1
> +a3
> +COMMIT;
> +SET @@autocommit=0;
> +CREATE TABLE t4 (s1 CHAR(2));
> +INSERT INTO t1 VALUES ('a4');
> +INSERT INTO t2 VALUES ('a4');
> +INSERT INTO t3 VALUES ('a4');
> +INSERT INTO t4 VALUES ('a4');
> +BACKUP DATABASE commit_test TO '84';
> +backup_id
> +#
> +RESTORE FROM '84';
> +backup_id
> +#
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +SELECT * FROM t4;
> +s1
> +a4
> +ROLLBACK;
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +SELECT * FROM t4;
> +s1
> +a4
> +COMMIT;
> +SET @@autocommit=1;
> +INSERT INTO t1 VALUES ('a5');
> +INSERT INTO t2 VALUES ('a5');
> +INSERT INTO t3 VALUES ('a5');
> +INSERT INTO t4 VALUES ('a5');
> +BACKUP DATABASE commit_test TO '85';
> +backup_id
> +#
> +RESTORE FROM '85';
> +backup_id
> +#
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +a5
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +a5
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +a5
> +SELECT * FROM t4;
> +s1
> +a4
> +a5
> +ROLLBACK;
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +a5
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +a5
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +a5
> +SELECT * FROM t4;
> +s1
> +a4
> +a5
> +COMMIT;
> +SET @@autocommit=1;
> +INSERT INTO t1 VALUES ('a6');
> +INSERT INTO t2 VALUES ('a6');
> +INSERT INTO t3 VALUES ('a6');
> +INSERT INTO t4 VALUES ('a6');
> +BACKUP DATABASE commit_test TO '86';
> +backup_id
> +#
> +SET @@autocommit=0;
> +RESTORE FROM '86';
> +backup_id
> +#
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +a5
> +a6
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +a5
> +a6
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +a5
> +a6
> +SELECT * FROM t4;
> +s1
> +a4
> +a5
> +a6
> +ROLLBACK;
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +a5
> +a6
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +a5
> +a6
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +a5
> +a6
> +SELECT * FROM t4;
> +s1
> +a4
> +a5
> +a6
> +COMMIT;
> +SET @@autocommit=0;
> +INSERT INTO t1 VALUES ('a7');
> +INSERT INTO t2 VALUES ('a7');
> +INSERT INTO t3 VALUES ('a7');
> +INSERT INTO t4 VALUES ('a7');
> +BACKUP DATABASE commit_test TO '87';
> +backup_id
> +#
> +SET @@autocommit=1;
> +RESTORE FROM '87';
> +backup_id
> +#
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +a5
> +a6
> +a7
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +a5
> +a6
> +a7
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +a5
> +a6
> +a7
> +SELECT * FROM t4;
> +s1
> +a4
> +a5
> +a6
> +a7
> +ROLLBACK;
> +SELECT * FROM t1;
> +s1
> +a1
> +a2
> +a3
> +a4
> +a5
> +a6
> +a7
> +SELECT * FROM t2;
> +s1
> +a2
> +a3
> +a4
> +a5
> +a6
> +a7
> +SELECT * FROM t3;
> +s1
> +a3
> +a4
> +a5
> +a6
> +a7
> +SELECT * FROM t4;
> +s1
> +a4
> +a5
> +a6
> +a7
> +COMMIT;
> +DROP DATABASE commit_test;
> +COMMIT;
> 
> === added file 'mysql-test/t/backup_commit_restore.test'
> --- a/mysql-test/t/backup_commit_restore.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/backup_commit_restore.test	2008-06-04 12:13:16 +0000
> @@ -0,0 +1,174 @@
> +--source include/have_innodb.inc
> +--source include/not_embedded.inc
> +
> +# Test that it is not possible to rollback restore.  That is, that an
> +# automatic commit is performed as part of restore.  This test both
> +# tests the drivers individually and in combination.  It also tests
> +# different combinations of turning autocommit on and off before
> +# backup and restore.  This test the behavior reported in BUG#34210
> +
> +CREATE DATABASE commit_test;
> +USE commit_test;
> +
> +# Test using InnoDB  (Consistent snapshot driver)
> +SET @@autocommit=0;
> +CREATE TABLE t1 (s1 CHAR(2)) ENGINE=innodb;
> +INSERT INTO t1 VALUES ('a1');
> +
> +replace_column 1 #;
> +BACKUP DATABASE commit_test TO '81';
> +
> +replace_column 1 #;
> +RESTORE FROM '81';
> +
> +SELECT * FROM t1;
> +ROLLBACK;
> +SELECT * FROM t1;
> +COMMIT;
> +
> +# Test using InnoDB  (Consistent snapshot driver) and Memory (default driver)
> +SET @@autocommit=0;
> +CREATE TABLE t2 (s1 CHAR(2)) ENGINE=Memory;
> +INSERT INTO t1 VALUES ('a2');
> +INSERT INTO t2 VALUES ('a2');
> +
> +replace_column 1 #;
> +BACKUP DATABASE commit_test TO '82';
> +
> +replace_column 1 #;
> +RESTORE FROM '82';
> +
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +ROLLBACK;
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +COMMIT;
> +
> +
> +# Test using InnoDB, Memory and MyISAM (Native driver)
> +SET @@autocommit=0;
> +CREATE TABLE t3 (s1 CHAR(2));
> +INSERT INTO t1 VALUES ('a3');
> +INSERT INTO t2 VALUES ('a3');
> +INSERT INTO t3 VALUES ('a3');
> +
> +replace_column 1 #;
> +BACKUP DATABASE commit_test TO '83';
> +
> +replace_column 1 #;
> +RESTORE FROM '83';
> +
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +ROLLBACK;
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +COMMIT;
> +
> +# Test using InnoDB, Memory, MyISAM and Falcon (Consistent Snapshot)
> +SET @@autocommit=0;
> +CREATE TABLE t4 (s1 CHAR(2));
> +INSERT INTO t1 VALUES ('a4');
> +INSERT INTO t2 VALUES ('a4');
> +INSERT INTO t3 VALUES ('a4');
> +INSERT INTO t4 VALUES ('a4');
> +
> +replace_column 1 #;
> +BACKUP DATABASE commit_test TO '84';
> +
> +replace_column 1 #;
> +RESTORE FROM '84';
> +
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +ROLLBACK;
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +COMMIT;
> +
> +
> +# Test doing backup/restore with auto-commit on
> +SET @@autocommit=1;
> +INSERT INTO t1 VALUES ('a5');
> +INSERT INTO t2 VALUES ('a5');
> +INSERT INTO t3 VALUES ('a5');
> +INSERT INTO t4 VALUES ('a5');
> +
> +replace_column 1 #;
> +BACKUP DATABASE commit_test TO '85';
> +
> +replace_column 1 #;
> +RESTORE FROM '85';
> +
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +ROLLBACK;
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +COMMIT;
> +
> +# Test doing backup with auto-commit on and restore with auto-commit off
> +SET @@autocommit=1;
> +INSERT INTO t1 VALUES ('a6');
> +INSERT INTO t2 VALUES ('a6');
> +INSERT INTO t3 VALUES ('a6');
> +INSERT INTO t4 VALUES ('a6');
> +
> +replace_column 1 #;
> +BACKUP DATABASE commit_test TO '86';
> +
> +SET @@autocommit=0;
> +replace_column 1 #;
> +RESTORE FROM '86';
> +
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +ROLLBACK;
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +COMMIT;
> +
> +
> +# Test doing backup with auto-commit off and restore with auto-commit on
> +SET @@autocommit=0;
> +INSERT INTO t1 VALUES ('a7');
> +INSERT INTO t2 VALUES ('a7');
> +INSERT INTO t3 VALUES ('a7');
> +INSERT INTO t4 VALUES ('a7');
> +
> +replace_column 1 #;
> +BACKUP DATABASE commit_test TO '87';
> +
> +SET @@autocommit=1;
> +replace_column 1 #;
> +RESTORE FROM '87';
> +
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +ROLLBACK;
> +SELECT * FROM t1;
> +SELECT * FROM t2;
> +SELECT * FROM t3;
> +SELECT * FROM t4;
> +COMMIT;
> +
> +# Clean-up
> +DROP DATABASE commit_test;
> +COMMIT;
> 
> === modified file 'sql/backup/data_backup.cc'
> --- a/sql/backup/data_backup.cc	2008-05-05 15:03:24 +0000
> +++ b/sql/backup/data_backup.cc	2008-06-04 12:13:16 +0000
> @@ -1597,6 +1597,15 @@
>    if (table_list)
>      close_thread_tables(::current_thd);
>  
> +  { // If auto commit is turned off, be sure to commit the transaction
> +    THD *thd=::current_thd;
> +    if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))
> +    {
> +      if (ha_autocommit_or_rollback(thd, 0)) state=ERROR;
> +      if (end_active_trans(thd)) state=ERROR;
> +    }
> +  }
> +
>    DBUG_RETURN(state == ERROR ? backup::ERROR : 0);
>  
>   error:
> 
>