Rafal,
I see in the code where you are capturing invalid paths for Windows, but it
still outputs the same error message for both invalid paths and overwrite
error. The following is what the code does now:
# attempt to backup to an invalid location on Windows:
mysql> backup database test to 'e:\\source\\c++\\back.bak';
ERROR 1614 (HY000): Can't write to backup location
'e:\source\c++\back.bak' <--- I don't have an e: drive
# attempt to backup to a valid location on Windows:
mysql> backup database test to 'd:\\source\\c++\\back.bak';
+-----------+
| backup_id |
+-----------+
| 72 |
+-----------+
1 row in set (3.03 sec)
# attempt to overwrite a backup file on Windows:
mysql> backup database test to 'd:\\source\\c++\\back.bak';
ERROR 1614 (HY000): Can't write to backup location
'd:\source\c++\back.bak'
I think this is confusing and doesn't clearly state what the problem is.
Notice the error for the third backup command. The error message is the same
as the first command and the user doesn't know whether the path or filename
is wrong or if he is trying to overwrite a file.
To illustrate, I think the above execution should result in the following:
# attempt to backup to an invalid location on Windows:
mysql> backup database test to 'e:\\source\\c++\\back.bak';
ERROR 1614 (HY000): Can't write to backup location
'e:\source\c++\back.bak'
^ Tells me something wrong with path or file ^
# attempt to backup to a valid location on Windows:
mysql> backup database test to 'd:\\source\\c++\\back.bak';
+-----------+
| backup_id |
+-----------+
| 72 |
+-----------+
1 row in set (3.03 sec)
# attempt to overwrite a backup file on Windows:
mysql> backup database test to 'd:\\source\\c++\\back.bak';
ERROR 1613 (HY000): Attempting to overwrite file 'd:\source\c++\back.bak'
^ Tells me I cannot overwrite the file and makes more sense (to me) ^
This gives the user more information to go on. The first invalid command
tells the user there is something wrong with the file/path and the third one
tells the user he is trying to overwrite the file (which is not allowed). Do
you agree this is more appropriate and accurate?
This can be accomplished using:
(kernel.cc)
+ if (!loc || !loc->is_valid())
+ {
+ my_error(ER_BACKUP_INVALID_LOC,MYF(0),lex->backup_dir.str);
+ DBUG_RETURN(ER_BACKUP_INVALID_LOC);
+ }
+ if (!my_access(lex->backup_dir.str,F_OK))
+ {
+ my_error(ER_BACKUP_CANNOT_OVERWRITE,MYF(0),lex->backup_dir.str);
+ DBUG_RETURN(ER_BACKUP_CANNOT_OVERWRITE);
+ }
(errmsg.txt)
+ ER_BACKUP_CANNOT_OVERWRITE
+ eng "Attempting to overwrite file '%-.64s'"
Note: This works on both Linux and Windows. :))
Otherwise, everything else in the patch looks fine. Depending on who pushes
first we may need to change my patch for BUG#33024 to add the remove file
command. But that's a minor thing and easy to do.
How about the other 2 bugs? Is this a "three-fer" where one patch fixes
three bugs?
Chuck
> -----Original Message-----
> From: rsomla@stripped [mailto:rsomla@stripped]
> Sent: Wednesday, December 12, 2007 15:24 PM
> To: commits@stripped
> Subject: bk commit into 6.0 tree (rafal:1.2748) BUG#33119
>
> Below is the list of changes that have just been committed
> into a local 6.0 repository of rafal. When rafal 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-12 21:23:23+01:00, rafal@quant.(none) +14 -0
> BUG#33119 (Online backup silently overwrites files).
>
> This patch makes backup to fail with error "Can't write to
> backup location
> '<path>'" if the file already exists. Backup tests are
> updated so that they
> don't try to overwrite existing files.
>
> Also, in case of failed BACKUP operation, the backup file
> is deleted if it was
> created at its beginning.
>
> mysql-test/r/backup_errors.result@stripped, 2007-12-12
> 21:23:18+01:00, rafal@quant.(none) +8 -0
> Results update.
>
> mysql-test/r/backup_no_data.result@stripped, 2007-12-12
> 21:23:18+01:00, rafal@quant.(none) +12 -4
> Results update.
>
> mysql-test/r/backup_progress.result@stripped, 2007-12-12
> 21:23:18+01:00, rafal@quant.(none) +1 -1
> Don't use default engine - it will be MyISAM which causes
> problems, currently.
>
> mysql-test/t/backup.test@stripped, 2007-12-12 21:23:19+01:00,
> rafal@quant.(none) +5 -2
> Add cleanup.
>
> mysql-test/t/backup_commit_blocker.test@stripped, 2007-12-12
> 21:23:19+01:00, rafal@quant.(none) +2 -0
> Ensure that there no backup files around at the beginning
> of the test.
>
> mysql-test/t/backup_ddl_blocker.test@stripped, 2007-12-12
> 21:23:19+01:00, rafal@quant.(none) +19 -2
> - Ensure that there no backup files around at the
> beginning of the test.
> - Since BACKUP don't overwrite existing files any more,
> remove each file before
> backing up into it.
>
> mysql-test/t/backup_errors.test@stripped, 2007-12-12
> 21:23:19+01:00, rafal@quant.(none) +28 -4
> Added test for situation when existing file is about to
> be overwritten.
>
> mysql-test/t/backup_fkey.test@stripped, 2007-12-12
> 21:23:19+01:00, rafal@quant.(none) +7 -2
> - Ensure that there no backup files around at the
> beginning of the test.
> - Since BACKUP don't overwrite existing files any more,
> remove each file before
> backing up into it.
>
> mysql-test/t/backup_no_data.test@stripped, 2007-12-12
> 21:23:19+01:00, rafal@quant.(none) +19 -10
> - Ensure that there no backup files around at the
> beginning of the test.
> - Since BACKUP don't overwrite existing files any more,
> remove each file before
> backing up into it.
> - Don't mess with test database.
>
> mysql-test/t/backup_progress.test@stripped, 2007-12-12
> 21:23:19+01:00, rafal@quant.(none) +7 -3
> - Ensure that there no backup files around at the
> beginning of the test.
> - Since BACKUP don't overwrite existing files any more,
> remove each file before
> backing up into it.
>
> mysql-test/t/backup_snapshot.test@stripped, 2007-12-12
> 21:23:19+01:00, rafal@quant.(none) +2 -0
> Ensure that there no backup files around at the beginning
> of the test.
>
> sql/backup/backup_kernel.h@stripped, 2007-12-12 21:23:19+01:00,
> rafal@quant.(none) +22 -0
> - Added is_valid() method to Location class.
> - Added remove() method to Location class.
>
> sql/backup/kernel.cc@stripped, 2007-12-12 21:23:19+01:00,
> rafal@quant.(none) +48 -3
> - Check if backup/restore location is valid using its
> is_valid() method and
> report error if it is not the case.
> - Added code to remove backup file in case of error.
> - Implementation of Location::is_valid() method for
> File_loc class.
> - Implementation of Location::remove() method for File_loc class.
>
> sql/backup/stream.cc@stripped, 2007-12-12 21:23:19+01:00,
> rafal@quant.(none) +1 -1
> Added O_EXCL flag when openning file for writting. This
> makes open fail if the file
> already exists.
>
> diff -Nrup a/mysql-test/r/backup_errors.result
> b/mysql-test/r/backup_errors.result
> --- a/mysql-test/r/backup_errors.result 2007-11-06
> 19:32:22 +01:00
> +++ b/mysql-test/r/backup_errors.result 2007-12-12
> 21:23:18 +01:00
> @@ -8,8 +8,16 @@ RESTORE FROM 'test.bak'; ERROR HY000: Can't
> read backup location 'test.bak'
> CREATE DATABASE adb;
> CREATE DATABASE bdb;
> +CREATE TABLE bdb.t1(a int) ENGINE=MEMORY;
> BACKUP DATABASE adb TO '';
> ERROR HY000: Invalid backup location ''
> +BACKUP DATABASE adb TO "bdb/t1.frm";
> +ERROR HY000: Can't write to backup location 'bdb/t1.frm'
> +BACKUP DATABASE adb TO "test.bak";
> +backup_id
> +#
> +BACKUP DATABASE adb TO "test.bak";
> +ERROR HY000: Can't write to backup location 'test.bak'
> DROP DATABASE IF EXISTS foo;
> Warnings:
> Note 1008 Can't drop database 'foo'; database doesn't exist
> 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
> 21:28:03 +01:00
> +++ b/mysql-test/r/backup_no_data.result 2007-12-12
> 21:23:18 +01:00
> @@ -1,5 +1,7 @@
> DROP DATABASE IF EXISTS empty_db;
> +DROP DATABASE IF EXISTS other_db;
> CREATE DATABASE empty_db;
> +CREATE DATABASE other_db;
> BACKUP DATABASE empty_db TO 'empty_db.bak'; backup_id # @@
> -7,11 +9,12 @@ BACKUP DATABASE * TO 'all.bak'; backup_id #
> DROP DATABASE empty_db; -DROP DATABASE test;
> +DROP DATABASE other_db;
> SHOW DATABASES;
> Database
> information_schema
> mysql
> +test
> RESTORE FROM 'all.bak';
> backup_id
> #
> @@ -20,9 +23,10 @@ Database
> information_schema
> empty_db
> mysql
> +other_db
> test
> DROP DATABASE empty_db;
> -DROP DATABASE test;
> +DROP DATABASE other_db;
> RESTORE FROM 'empty_db.bak';
> backup_id
> #
> @@ -31,6 +35,7 @@ Database
> information_schema
> empty_db
> mysql
> +test
> SHOW TABLES IN empty_db;
> Tables_in_empty_db
> RESTORE FROM 'all.bak';
> @@ -41,10 +46,11 @@ Database
> information_schema
> empty_db
> mysql
> +other_db
> test
> SHOW TABLES IN empty_db;
> Tables_in_empty_db
> -USE test;
> +USE other_db;
> DROP TABLE IF EXISTS t1;
> Warnings:
> Note 1051 Unknown table 't1'
> @@ -54,7 +60,7 @@ CREATE TABLE t1 (
> ) ENGINE=MEMORY DEFAULT CHARSET=latin1; USE empty_db; DROP
> VIEW IF EXISTS v1; -CREATE VIEW v1 AS SELECT * FROM test.t1;
> +CREATE VIEW v1 AS SELECT * FROM other_db.t1;
> BACKUP DATABASE empty_db TO 'empty_db.bak'; backup_id # @@
> -63,6 +69,7 @@ Database information_schema empty_db mysql
> +other_db
> test
> RESTORE FROM 'empty_db.bak';
> backup_id
> @@ -71,3 +78,4 @@ USE empty_db;
> SHOW TABLES;
> Tables_in_empty_db
> DROP DATABASE IF EXISTS empty_db;
> +DROP DATABASE IF EXISTS other_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
> 18:38:02 +01:00
> +++ b/mysql-test/r/backup_progress.result 2007-12-12
> 21:23:18 +01:00
> @@ -6,7 +6,7 @@ con1: Create table and new users.
> CREATE TABLE backup_progress.t1 (a char(30)) ENGINE=MEMORY;
> 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);
> +CREATE TABLE backup_progress.t1_res (id INT) ENGINE=MEMORY;
> INSERT INTO backup_progress.t1 VALUES ("01 Test #1 -
> progress"); INSERT INTO backup_progress.t1 VALUES ("02 Test
> #1 - progress"); INSERT INTO backup_progress.t1 VALUES ("03
> Test #1 - progress"); diff -Nrup a/mysql-test/t/backup.test
> b/mysql-test/t/backup.test
> --- a/mysql-test/t/backup.test 2007-12-03 21:28:06 +01:00
> +++ b/mysql-test/t/backup.test 2007-12-12 21:23:19 +01:00
> @@ -7,6 +7,9 @@ connection backup;
>
> DROP DATABASE IF EXISTS db1;
> DROP DATABASE IF EXISTS db2;
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_default.bak
> +
>
> CREATE DATABASE db1;
> CREATE DATABASE db2;
> @@ -268,7 +271,7 @@ BACKUP DATABASE bup_default TO "bup_defa
>
> DROP DATABASE bup_default;
>
> ---replace_column 1 # 2 # 3 # 4 # 10 # 11 # 12 #
> +--replace_column 1 #
> --query_vertical RESTORE FROM "bup_default.bak"
>
> # Show the data
> @@ -284,5 +287,5 @@ SELECT COUNT(*) FROM bup_default.wide;
> DROP DATABASE IF EXISTS bup_default; --enable_warnings
>
> -#--exec rm $MYSQLTEST_VARDIR/master-data/bup_default.bak
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_default.bak
>
> diff -Nrup a/mysql-test/t/backup_commit_blocker.test
> b/mysql-test/t/backup_commit_blocker.test
> --- a/mysql-test/t/backup_commit_blocker.test 2007-12-06
> 19:05:55 +01:00
> +++ b/mysql-test/t/backup_commit_blocker.test 2007-12-12
> 21:23:19 +01:00
> @@ -47,6 +47,8 @@
>
> --disable_warnings
> DROP DATABASE IF EXISTS bup_commit_blocker;
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_commit_blocker.bak;
> --enable_warnings
>
> CREATE DATABASE bup_commit_blocker;
> 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-06
> 19:05:56 +01:00
> +++ b/mysql-test/t/backup_ddl_blocker.test 2007-12-12
> 21:23:19 +01:00
> @@ -55,6 +55,15 @@
> --source include/not_valgrind.inc
>
> #
> +# Remove backup files (if they exist)
> +#
> +
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker_orig.bak;
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker.bak;
> +
> +#
> # Connections used in this test
> #
> # con1 used to create data, load data, and run the backup
> @@ -456,6 +465,7 @@ send REPAIR TABLE bup_ddl_blocker.t2;
> connection con1;
>
> --echo con1: Backing up database -- will block with lock
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker.bak
> send BACKUP DATABASE bup_ddl_blocker TO "bup_ddl_blocker.bak";
>
> connection con6;
> @@ -595,6 +605,7 @@ SHOW TABLES;
> --source include/backup_ddl_create_data.inc
>
> # Get a backup to work with.
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker_orig.bak
> --replace_column 1 #
> BACKUP DATABASE bup_ddl_blocker to 'bup_ddl_blocker_orig.bak';
>
> @@ -764,6 +775,7 @@ send DROP TABLE bup_ddl_blocker.t2;
> connection con1;
>
> --echo con1: Backing up database -- will block with lock
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker.bak
> send BACKUP DATABASE bup_ddl_blocker TO "bup_ddl_blocker.bak";
>
> connection con6;
> @@ -900,6 +912,7 @@ SHOW TABLES;
> --source include/backup_ddl_create_data.inc
>
> # Get a backup to work with.
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker_orig.bak
> --replace_column 1 #
> BACKUP DATABASE bup_ddl_blocker to 'bup_ddl_blocker_orig.bak';
>
> @@ -1095,6 +1108,7 @@ send DROP DATABASE bup_ddl_blocker_2;
> connection con1;
>
> --echo con1: Backing up database -- will block with lock
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker.bak
> send BACKUP DATABASE * TO "bup_ddl_blocker.bak";
>
> connection con6;
> @@ -1260,6 +1274,7 @@ INSERT INTO bup_ddl_blocker_4.t1 VALUES
> SHOW DATABASES LIKE 'bup_ddl_blocker_%';
>
> # Get a backup to work with.
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker_orig.bak
> --replace_column 1 #
> BACKUP DATABASE bup_ddl_blocker_2, bup_ddl_blocker_4 to
> 'bup_ddl_blocker_orig.bak';
>
> @@ -1455,6 +1470,7 @@ send ALTER DATABASE bup_ddl_blocker_2
> CH connection con1;
>
> --echo con1: Backing up database -- will block with lock
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker.bak
> send BACKUP DATABASE * TO "bup_ddl_blocker.bak";
>
> connection con6;
> @@ -1642,6 +1658,7 @@ INSERT INTO bup_ddl_blocker_4.t1 VALUES
> SHOW DATABASES LIKE 'bup_ddl_blocker_%';
>
> # Get a backup to work with.
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker_orig.bak
> --replace_column 1 #
> BACKUP DATABASE bup_ddl_blocker_2, bup_ddl_blocker_4 to
> 'bup_ddl_blocker_orig.bak';
>
> @@ -1951,8 +1968,8 @@ DROP TABLE test.t2; --echo con1:
> Cleanup DROP DATABASE bup_ddl_blocker;
>
> -remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker_orig.bak;
> -remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker.bak;
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker_orig.bak
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_ddl_blocker.bak
>
>
>
> diff -Nrup a/mysql-test/t/backup_errors.test
> b/mysql-test/t/backup_errors.test
> --- a/mysql-test/t/backup_errors.test 2007-11-30 05:22:00 +01:00
> +++ b/mysql-test/t/backup_errors.test 2007-12-12 21:23:19 +01:00
> @@ -1,20 +1,41 @@
> --source include/not_embedded.inc
>
> +# Check that BACKUP/RESTORE commands correctly report errors
> # # TODO:
> +When we know how to do that, check that the backup progress table #
> +contains appropriate rows when errors have been detected.
> +
> DROP DATABASE IF EXISTS adb;
> DROP DATABASE IF EXISTS bdb;
>
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/test.bak
> +
> # non-existent backup archive
> --- exec rm -f $MYSQLTEST_VARDIR/master-data/test.bak
> -- error ER_BACKUP_READ_LOC
> RESTORE FROM 'test.bak';
>
> CREATE DATABASE adb;
> CREATE DATABASE bdb;
> +CREATE TABLE bdb.t1(a int) ENGINE=MEMORY;
>
> -# bad archive name
> --- error ER_BACKUP_INVALID_LOC
> +# invalid location
> +--error ER_BACKUP_INVALID_LOC
> BACKUP DATABASE adb TO '';
>
> +# don't overwrite existing files
> +--error ER_BACKUP_WRITE_LOC
> +BACKUP DATABASE adb TO "bdb/t1.frm";
> +
> +--replace_column 1 #
> +BACKUP DATABASE adb TO "test.bak";
> +
> +# don't overwrite existing backup image --error ER_BACKUP_WRITE_LOC
> +BACKUP DATABASE adb TO "test.bak";
> +
> +--remove_file $MYSQLTEST_VARDIR/master-data/test.bak
> +
> # non-existent database
> DROP DATABASE IF EXISTS foo;
> DROP DATABASE IF EXISTS bar;
> @@ -28,4 +49,7 @@ BACKUP DATABASE test,foo,bdb,bar TO 'tes
> DROP DATABASE IF EXISTS adb; DROP DATABASE IF EXISTS bdb;
>
> -
> +# Note: the file should be removed - if it is not, the following
> +command # will fail and we will be warned that something is
> wrong with
> +the test --error 1 --remove_file
> $MYSQLTEST_VARDIR/master-data/test.bak
> diff -Nrup a/mysql-test/t/backup_fkey.test
> b/mysql-test/t/backup_fkey.test
> --- a/mysql-test/t/backup_fkey.test 2007-11-30 05:22:00 +01:00
> +++ b/mysql-test/t/backup_fkey.test 2007-12-12 21:23:19 +01:00
> @@ -14,6 +14,10 @@
>
> --disable_warnings
> DROP DATABASE IF EXISTS backup_fkey;
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_fkey.bak;
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_fkey_orig.bak;
> --enable_warnings
>
> CREATE DATABASE backup_fkey;
> @@ -105,6 +109,7 @@ INSERT INTO backup_fkey.t1 VALUES ("06 T
> INSERT INTO backup_fkey.t1 VALUES ("07 Test #2 - foreign key
> constraints");
>
> --echo Backup the data
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_fkey.bak
> --replace_column 1 #
> BACKUP DATABASE backup_fkey TO 'backup_fkey.bak';
>
> @@ -141,6 +146,6 @@ SHOW VARIABLES LIKE 'foreign_key_checks%
>
> DROP DATABASE backup_fkey;
>
> -remove_file $MYSQLTEST_VARDIR/master-data/backup_fkey.bak;
> -remove_file $MYSQLTEST_VARDIR/master-data/backup_fkey_orig.bak;
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_fkey.bak
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_fkey_orig.bak
>
> 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
> 21:28:08 +01:00
> +++ b/mysql-test/t/backup_no_data.test 2007-12-12
> 21:23:19 +01:00
> @@ -4,22 +4,27 @@ connect (backup,localhost,root,,);
>
> connection backup;
>
> -# save empty test database on the side
> ---exec cp -rf $MYSQLTEST_VARDIR/master-data/test
> $MYSQLTEST_VARDIR/test.orig
>
> --disable_warnings
> DROP DATABASE IF EXISTS empty_db;
> +DROP DATABASE IF EXISTS other_db;
> --enable_warnings
>
> CREATE DATABASE empty_db;
> +CREATE DATABASE other_db;
>
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/empty_db.bak
> --replace_column 1 #
> BACKUP DATABASE empty_db TO 'empty_db.bak';
> +
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/all.bak
> --replace_column 1 #
> BACKUP DATABASE * TO 'all.bak';
>
> DROP DATABASE empty_db;
> -DROP DATABASE test;
> +DROP DATABASE other_db;
>
> SHOW DATABASES;
>
> @@ -29,7 +34,7 @@ RESTORE FROM 'all.bak'; SHOW DATABASES;
>
> DROP DATABASE empty_db;
> -DROP DATABASE test;
> +DROP DATABASE other_db;
>
> --replace_column 1 #
> RESTORE FROM 'empty_db.bak';
> @@ -45,7 +50,7 @@ SHOW TABLES IN empty_db;
>
> connection backup;
>
> -USE test;
> +USE other_db;
>
> DROP TABLE IF EXISTS t1;
>
> @@ -60,8 +65,10 @@ USE empty_db;
> DROP VIEW IF EXISTS v1;
> --enable_warnings
>
> -CREATE VIEW v1 AS SELECT * FROM test.t1;
> +CREATE VIEW v1 AS SELECT * FROM other_db.t1;
>
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/empty_db.bak
> --replace_column 1 #
> BACKUP DATABASE empty_db TO 'empty_db.bak';
>
> @@ -74,9 +81,11 @@ USE empty_db;
>
> SHOW TABLES;
> DROP DATABASE IF EXISTS empty_db;
> +DROP DATABASE IF EXISTS other_db;
> +
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/all.bak
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/empty_db.bak
>
> -# restore test database to the exact state from the
> beginning of this test ---exec rm -rf
> $MYSQLTEST_VARDIR/master-data/test/*
> ---exec cp -rf $MYSQLTEST_VARDIR/test.orig
> $MYSQLTEST_VARDIR/master-data/test
> ---exec rm -rf $MYSQLTEST_VARDIR/test.orig/*
>
> 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
> 22:42:58 +01:00
> +++ b/mysql-test/t/backup_progress.test 2007-12-12
> 21:23:19 +01:00
> @@ -12,6 +12,8 @@
> --disable_warnings
> DROP DATABASE IF EXISTS backup_progress; DROP TABLE IF
> EXISTS backup_progress.t1_res;
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_progress_orig.bak;
> --enable_warnings
>
> connect (con1,localhost,root,,);
> @@ -33,7 +35,7 @@ CREATE DATABASE backup_progress; CREATE
> TABLE backup_progress.t1 (a char(30)) ENGINE=MEMORY; 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);
> +CREATE TABLE backup_progress.t1_res (id INT) ENGINE=MEMORY;
>
> INSERT INTO backup_progress.t1 VALUES ("01 Test #1 -
> progress"); INSERT INTO backup_progress.t1 VALUES ("02 Test
> #1 - progress"); @@ -229,10 +231,12 @@ SELECT obp.* FROM
> mysql.online_backup_pr connection con1;
>
> --replace_column 1 #
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_progress_orig.bak
> --error 1049
> BACKUP DATABASE DOES_NOT_EXIST to 'backup_progress_orig.bak';
>
> DROP DATABASE backup_progress;
> -
> -remove_file $MYSQLTEST_VARDIR/master-data/backup_progress_orig.bak;
> +# note: the backup file should be deleted.
> +--error 1
> +--remove_file $MYSQLTEST_VARDIR/master-data/backup_progress_orig.bak
>
> diff -Nrup a/mysql-test/t/backup_snapshot.test
> b/mysql-test/t/backup_snapshot.test
> --- a/mysql-test/t/backup_snapshot.test 2007-12-06
> 19:05:57 +01:00
> +++ b/mysql-test/t/backup_snapshot.test 2007-12-12
> 21:23:19 +01:00
> @@ -33,6 +33,8 @@
>
> --disable_warnings
> DROP DATABASE IF EXISTS bup_snapshot;
> +--error 0,1
> +--remove_file $MYSQLTEST_VARDIR/master-data/bup_snapshot.bak;
> --enable_warnings
>
> CREATE DATABASE bup_snapshot;
> diff -Nrup a/sql/backup/backup_kernel.h b/sql/backup/backup_kernel.h
> --- a/sql/backup/backup_kernel.h 2007-11-30 21:14:08 +01:00
> +++ b/sql/backup/backup_kernel.h 2007-12-12 21:23:19 +01:00
> @@ -65,9 +65,31 @@ struct Location
> virtual void free()
> { delete this; } // use destructor to free resources
>
> + /**
> + Determine if the location is valid
> +
> + An invalid location will not be used.
> +
> + @retval TRUE Location is valid.
> + @retval FALSE Location is invalid.
> + */
> + virtual bool is_valid() =0;
> +
> /// Describe location for debug purposes
> virtual const char* describe()
> { return "Invalid location"; }
> +
> + /**
> + Remove any system resources connected to that location.
> +
> + When location is opened for writing, some system
> resources will be usually
> + created (depending on the type of the location). For
> example, a file in the
> + file system. This method should remove/free any such
> resources. If no
> + resources were created, the method should do nothing.
> +
> + @returns OK on success, ERROR otherwise.
> + */
> + virtual result_t remove() =0;
>
> /**
> Interpret string passed to BACKUP/RESTORE statement as
> backup location diff -Nrup a/sql/backup/kernel.cc
> b/sql/backup/kernel.cc
> --- a/sql/backup/kernel.cc 2007-12-03 21:28:16 +01:00
> +++ b/sql/backup/kernel.cc 2007-12-12 21:23:19 +01:00
> @@ -103,7 +103,7 @@ execute_backup_command(THD *thd, LEX *le
>
> Location *loc= Location::find(lex->backup_dir);
>
> - if (!loc)
> + if (!loc || !loc->is_valid())
> {
> my_error(ER_BACKUP_INVALID_LOC,MYF(0),lex->backup_dir.str);
> DBUG_RETURN(ER_BACKUP_INVALID_LOC);
> @@ -246,10 +246,10 @@ execute_backup_command(THD *thd, LEX *le
>
> case SQLCOM_BACKUP:
> {
> + /* if set to true, backup location will be removed
> (e.g., upon failure) */
> + bool remove_location= FALSE;
> backup::OStream *stream= open_for_write(*loc);
>
> - // TODO: report error if location exists
> -
> if (!stream)
> {
> my_error(ER_BACKUP_WRITE_LOC,MYF(0),loc->describe());
> @@ -352,6 +352,15 @@ execute_backup_command(THD *thd, LEX *le
> if (stop)
> report_ob_time(backup_prog_id, 0, stop);
>
> + /*
> + If the output stream was opened, a file or other
> system resource
> + (depending on the type of the backup location) could
> be created. Since
> + we failed to write backup image, this resource should
> be removed.
> + We set remove_location flag so that loc->remove() will
> be called after
> + closing the output stream.
> + */
> + remove_location= TRUE;
> +
> BACKUP_BREAKPOINT("bp_error_state");
>
> finish_backup:
> @@ -363,8 +372,14 @@ execute_backup_command(THD *thd, LEX *le
> BACKUP_BREAKPOINT("DDL_unblocked");
>
> if (stream)
> + {
> stream->close();
>
> + if (remove_location)
> + if (loc->remove() != OK)
> + res= res ? res : ERROR;
> + }
> +
> break;
> }
>
> @@ -1608,9 +1623,39 @@ struct File_loc: public Location
> File_loc(const char *p)
> { path.append(p); }
>
> + bool is_valid()
> + {
> + /*
> + On some systems certain file names are invalid. We use
> + check_if_legal_filename() function from mysys to detect this.
> + */
> +#if defined(__WIN__) || defined(__EMX__)
> +
> + if (check_if_legal_filename(path.c_ptr()))
> + return FALSE;
> +
> +#endif
> + return TRUE;
> + }
> +
> const char* describe()
> { return path.c_ptr(); }
>
> + result_t remove()
> + {
> + int res= my_delete(path.c_ptr(),MYF(0));
> +
> + /*
> + Ignore ENOENT error since it is ok if the file doesn't exist.
> + */
> + if (my_errno == ENOENT)
> + res= 0;
> +
> + if (res)
> + sql_print_error(ER(ER_CANT_DELETE_FILE),path.c_ptr(),my_errno);
> +
> + return res ? ERROR : OK;
> + }
> };
>
>
> diff -Nrup a/sql/backup/stream.cc b/sql/backup/stream.cc
> --- a/sql/backup/stream.cc 2007-11-30 09:23:30 +01:00
> +++ b/sql/backup/stream.cc 2007-12-12 21:23:19 +01:00
> @@ -147,7 +147,7 @@ bool Stream::rewind()
>
>
> OStream::OStream(const ::String &name):
> - Stream(name,O_WRONLY|O_CREAT|O_TRUNC), bytes(0)
> + Stream(name,O_WRONLY|O_CREAT|O_EXCL|O_TRUNC), bytes(0)
> {
> stream.write= stream_write;
> m_block_size=0; // use default block size provided by the
> backup stram library
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1
>
