Re: bzr commit into mysql-6.0-backup branch (jorgen.loland:2727) Bug#34579

[Rafal Somla]

STATUS
------
Changes requested.

REQUESTS
--------
1. Add one test scenario: RESTORE without OVERWRITE flag in the situation when 
restore database do not exist (so that the operation should succeed).

SUGGESTIONS
-----------

1. Use existing backup and backup_errors test for testing this functionality.
2. Do not pass bool value via a pointer but directly.

COMMENTARY
----------
I run backup suite with patched code and all tests passed, except for 
backup_no_be test which is not included in your patch. I assume that you will 
fix this when you update your tree before the final push.

DETAILS
-------
Jorgen Loland wrote:
> #At file:///localhome/jl208045/mysql/mysql-6.0-backup-34579/
> 
>  2727 Jorgen Loland	2008-11-07
>       Bug#34579 - Backup: Restore overwrites the new / modified data without warning
>       
>       Add OVERWRITE flag to RESTORE command
>       
>       Before, RESTORE would overwrite existing DBs with same name. With this patch,
> RESTORE will error if database exists and OVERWRITE is not specified.
> added:
>   mysql-test/suite/backup/r/backup_restore_overwrite.result
>   mysql-test/suite/backup/t/backup_restore_overwrite.test
> modified:
>   mysql-test/lib/mtr_report.pl
>   mysql-test/suite/backup/r/backup.result
>   mysql-test/suite/backup/r/backup_backupdir.result
>   mysql-test/suite/backup/r/backup_commit_backup.result
>   mysql-test/suite/backup/r/backup_commit_blocker.result
>   mysql-test/suite/backup/r/backup_commit_restore.result
>   mysql-test/suite/backup/r/backup_compression.result
>   mysql-test/suite/backup/r/backup_concurrent.result
>   mysql-test/suite/backup/r/backup_db_grants.result
>   mysql-test/suite/backup/r/backup_fkey.result
>   mysql-test/suite/backup/r/backup_lock_myisam.result
>   mysql-test/suite/backup/r/backup_logs.result
>   mysql-test/suite/backup/r/backup_logs_purge.result
>   mysql-test/suite/backup/r/backup_security.result
>   mysql-test/suite/backup/r/backup_snapshot.result
>   mysql-test/suite/backup/t/backup.test
>   mysql-test/suite/backup/t/backup_backupdir.test
>   mysql-test/suite/backup/t/backup_commit_backup.test
>   mysql-test/suite/backup/t/backup_commit_blocker.test
>   mysql-test/suite/backup/t/backup_commit_restore.test
>   mysql-test/suite/backup/t/backup_compression.test
>   mysql-test/suite/backup/t/backup_concurrent.test
>   mysql-test/suite/backup/t/backup_db_grants.test
>   mysql-test/suite/backup/t/backup_fkey.test
>   mysql-test/suite/backup/t/backup_lock_myisam.test
>   mysql-test/suite/backup/t/backup_logs.test
>   mysql-test/suite/backup/t/backup_logs_purge.test
>   mysql-test/suite/backup/t/backup_security.test
>   mysql-test/suite/backup/t/backup_snapshot.test
>   mysql-test/suite/rpl/r/rpl_backup.result
>   mysql-test/suite/rpl/t/rpl_backup.test
>   sql/backup/backup_kernel.h
>   sql/backup/kernel.cc
>   sql/lex.h
>   sql/share/errmsg.txt
>   sql/sql_parse.cc
>   sql/sql_yacc.yy
>

(cut)
> === added file 'mysql-test/suite/backup/t/backup_restore_overwrite.test'
> --- a/mysql-test/suite/backup/t/backup_restore_overwrite.test	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/backup/t/backup_restore_overwrite.test	2008-11-07 11:31:08
> +0000
> @@ -0,0 +1,34 @@
> +#
> +# This test is designed to test the RESTORE ... OVERWRITE functionality
> +#
> +
> +--source include/not_embedded.inc
> +
> +--echo Initialize
> +CREATE DATABASE overwrite_bup;
> +USE overwrite_bup;
> +CREATE TABLE table1 (text VARCHAR(20));
> +INSERT INTO table1 VALUES ('Inserted before');
> +
> +--echo 
> +--echo Backup database
> +--replace_column 1 #
> +BACKUP DATABASE overwrite_bup TO 'overwrite.bak';
> +
> +--echo 
> +--echo Insert more data and display
> +INSERT INTO table1 VALUES ('Inserted after');
> +SELECT * FROM table1;
> +
> +--echo 
> +--echo Restore without overwrite; will fail
> +--error ER_RESTORE_DB_EXISTS
> +RESTORE FROM 'overwrite.bak';
> +
> +--echo 
> +--replace_column 1 #
> +RESTORE FROM 'overwrite.bak' OVERWRITE;
> +
> +--echo 
> +--echo Show that inserted value 2 is not there
> +SELECT * FROM table1;
> 

Suggestion: testing error if no OVERWRITE in backup_errors, testing correct 
restore if OVERWRITE present in the main backup test.

Request: Test that RESTORE ... OVERWRITE works fine if restored databases do not 
exist.

(cut)
> === modified file 'sql/backup/backup_kernel.h'
> --- a/sql/backup/backup_kernel.h	2008-10-30 17:53:24 +0000
> +++ b/sql/backup/backup_kernel.h	2008-11-07 11:31:08 +0000
> @@ -30,7 +30,7 @@ void backup_shutdown();
>    Called from the big switch in mysql_execute_command() to execute
>    backup related statement
>  */
> -int execute_backup_command(THD*, LEX*, String*);
> +int execute_backup_command(THD*, LEX*, String*, bool*);

Suggestion: pass bool directly, not via a pointer.

>  
>  // forward declarations
>  
> @@ -74,7 +74,7 @@ class Backup_restore_ctx: public backup:
>                                     const char*);  
>  
>    int do_backup();
> -  int do_restore();
> +  int do_restore(bool *overwrite);

Suggestion: pass bool directly, not via a pointer.

>    int fatal_error(int, ...);
>    int log_error(int, ...);
>  
> 

Rafal