Hi Thava,
Status: Approved.
Options: In the details I have a few minor comments about commenting
style, which reflect my personal preference. I would be happy if you
follow them, but this is not required.
DETAILS:
Thava Alagu, 15.10.2009 03:48:
...
> 2881 Thava Alagu 2009-10-15
> BUG#36402 - Backup needs option to overwrite backup image
>
> The backup sql command now takes additional option "OVERWRITE"
> to overwrite backup image file even if one exists with the same name.
...
> === modified file 'sql/sql_parse.cc'
...
> @@ -2401,8 +2401,11 @@ mysql_execute_command(THD *thd)
> sys_var_backupdir.value_length);
> backupdir.length(sys_var_backupdir.value_length);
>
> - /* Used to specify if RESTORE should overwrite existing db with same name */
> - bool overwrite_restore= false;
> + /*
> + overwrite for RESTORE overwrites existing db with same name.
> + overwrite for BACKUP overwrites existing backup image.
I would prefer these sentences to start with a capital letter.
> + */
> + bool overwrite_option= false;
>
> /* Used to specify if RESTORE should skup writing the gap event. */
> bool skip_gap_event= false;
> @@ -2410,8 +2413,7 @@ mysql_execute_command(THD *thd)
> List<Item> lit= lex->value_list;
> Item *it= 0;
>
> - // value list only set for RESTORE in sql_yacc.yy, no error checking of
> - // item necessary for backup
> + // value list set for BACKUP and RESTORE options in sql_yacc.yy
Since you changed the comment anyway: I would prefer this to be a full
sentence, starting with a capital letter and ending with a dot.
Ideally it would be a C style comment as it is disputable, if C++ style
is allowed for a comment-only line.
...
> === modified file 'sql/sql_yacc.yy'
...
> @@ -6817,6 +6817,7 @@ backup:
> TO_SYM
> TEXT_STRING_sys
> opt_compression
> + opt_overwrite
> {
> Lex->backup_dir = $6;
> }
Just for your information (no need to change): That way we require a
certain order of options for BACKUP like we do also for RESTORE. This is
relevant for the documentation of the new option.
Please add a note for the docs team to the bug report when you push the
patch.
Regards
Ingo
--
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring HRB München 161028
