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

[Chuck Bell]

STATUS
------
Patch approved pending fix for warning in sql_yacc.yy and consideration of
proposal.

REQUESTS
--------
1. There is a warning during the compilation of the yacc code. See details
for the solution.

sql_yacc.yy:6425.11-6441.23: warning: type clash on default action: <NONE>
!= <>

2. (proposal) In addition to forcing the user to specify OVERWRITE on
restore, what about adding a new session variable named
'overwrite_on_restore' which defaults to FALSE. This will allow DBA's who
want to do a bunch of restores to set the variable and off they go without
having to type OVERWRITE again and again. It is relatively harmless to add.
Changes to the code would be something like this:

+  // unless RESTORE... OVERWRITE: return error if database already exists
+  if (!overwrite && !sys_var_overwrite_restore.value) {
+    Image_info::Db_iterator *dbit= info.get_dbs();
+
+    if (!dbit) {
+      DBUG_RETURN(fatal_error(ER_OUT_OF_RESOURCES));
+    }
+
+    Image_info::Db *mydb;
+    while (mydb= static_cast<Image_info::Db*>((*dbit)++)) {
+      if (!obs::check_db_existence(&mydb->name())) {
+        DBUG_RETURN(fatal_error(ER_RESTORE_DB_EXISTS, mydb->name().ptr()));
+      }
+    }
+    delete dbit;
+  }

...plus the code for the variable. ;)

If you think this is a good idea, I think we should either open a new bug
report (feature request) and add it or add it to this patch. Either way, I
think this will be a welcome addition.
 
COMMENTARY
----------
Code looks good otherwise.

SUGGESTIONS
-----------
None

DETAILS
-------
>  2727 Jorgen Loland	2008-11-10
>       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.

...

> @@ -6437,8 +6438,28 @@ restore:
>            {
>              Lex->backup_dir = $4; 
>            }
> +          opt_overwrite
>          ;
>  
> +opt_overwrite:
> +          /* empty */ 
> +          {         
> +            LEX *lex= Lex;
> +            Item *it= new Item_int((int8) 0);
> +
> +            lex->value_list.empty();
> +            lex->value_list.push_front(it);
> +          }
> +        | OVERWRITE_SYM 
> +          {
> +            LEX *lex= Lex;
> +            Item *it= new Item_int((int8) 1);
> +
> +            lex->value_list.empty();
> +            lex->value_list.push_front(it);
> +           }
> +         ;
> +

[1] The warning is caused by a misplaced subdeclaration. Change the patch
thusly:

> @@ -6437,8 +6438,28 @@ restore:
> -          TEXT_STRING_sys                  <-------- HERE 
> +          TEXT_STRING_sys opt_overwrite    <-------- HERE 
>            {
>              Lex->backup_dir = $4; 
>            }
>          ;

...

Chuck