Hi Chuck,
Before implementing COMMENT option let's agree on its semantics. Your patch
implements the one in which BACKUP ... COMMENT "Foo" or RESTORE ... COMMENT
"Foo" simply adds the provided comment to backup/restore logs.
But I think the correct semantics of this option should be that it writes the
given comment to the produced backup image file (and also logs it in the logs).
Then RESTORE should read and log the comment from the backup image. Since
RESTORE reads comment from the image, there is no point in adding COMMENT option
to it. A use scenario would be:
sql> BACKUP ... TO 'image.bkp' COMMENT "My data after upgrade"
Now the backup image is created and comment "My data after upgrade" is stored
inside it as well as reported in backup_history log. Then, few months later:
sql> RESTORE FROM 'image.bkp';
Which will restore data from the image and report in backup_history the comment
"My data after upgrade" stored there.
To implement this semantics, the backup image format must be updated to provide
space for storing comments.
Rafal
Chuck Bell wrote:
> #At file:///C:/source/bzr/mysql-6.0-bug-43515/ based on
> revid:charles.bell@stripped
>
> 2801 Chuck Bell 2009-03-15
> BUG#43515 : There is no way to use user_comment option from backup_history log
>
> This patch adds the user_comment to the RESTORE and BACKUP
> commands.
> modified:
> mysql-test/suite/backup/r/backup_logs.result
> mysql-test/suite/backup/t/backup_logs.test
> sql/backup/backup_kernel.h
> sql/backup/kernel.cc
> sql/backup/logger.h
> sql/si_logs.cc
> sql/si_logs.h
> sql/sql_yacc.yy
>
> per-file messages:
> mysql-test/suite/backup/r/backup_logs.result
> Corrected result file.
> mysql-test/suite/backup/t/backup_logs.test
> Added test cases to test user_comment from command line.
> sql/backup/backup_kernel.h
> Added parameter for user comment.
> sql/backup/kernel.cc
> Added parameter to user comment for recording to the log.
> sql/backup/logger.h
> Added method to save user comment.
> sql/si_logs.cc
> Added methods to save comment from command line.
> sql/si_logs.h
> Extended classes to include saving user comment from command line.
> sql/sql_yacc.yy
> Extended BACKUP and RESTORE commands to include user comment.
> === modified file 'mysql-test/suite/backup/r/backup_logs.result'
> --- a/mysql-test/suite/backup/r/backup_logs.result 2009-02-26 22:19:09 +0000
> +++ b/mysql-test/suite/backup/r/backup_logs.result 2009-03-15 19:57:33 +0000
> @@ -62,7 +62,7 @@ con1: Activate sync points for the backu
> SET DEBUG_SYNC= 'after_backup_log_init SIGNAL started WAIT_FOR do_run';
> SET DEBUG_SYNC= 'after_backup_start_backup SIGNAL running WAIT_FOR finish';
> Perform backup database operation with database alone.
> -BACKUP DATABASE backup_logs TO 'backup_logs1.bak';
> +BACKUP DATABASE backup_logs TO 'backup_logs1.bak' COMMENT "Test 2";
> con default: Wait for the backup to be started.
> SET DEBUG_SYNC= 'now WAIT_FOR started';
> Let backup step to running state.
> @@ -75,10 +75,10 @@ backup_id
> Get last backup_id
> SELECT MAX(backup_id) INTO @bup_id FROM mysql.backup_history
> WHERE command LIKE "BACKUP DATABASE backup_logs TO%";
> -SELECT operation,num_objects, username, command FROM mysql.backup_history
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> WHERE backup_id=@bup_id;
> -operation num_objects username command
> -backup 1 tom BACKUP DATABASE backup_logs TO 'backup_logs1.bak'
> +operation num_objects username command user_comment
> +backup 1 tom BACKUP DATABASE backup_logs TO 'backup_logs1.bak' COMMENT "Test 2"
> SELECT CURRENT_USER();
> CURRENT_USER()
> root@localhost
> @@ -89,10 +89,10 @@ backup_id
> Get last backup_id
> SELECT MAX(backup_id) INTO @bup_id FROM mysql.backup_history
> WHERE command LIKE "BACKUP DATABASE backup_logs TO%";
> -SELECT operation,num_objects, username, command FROM mysql.backup_history
> +SELECT operation,num_objects, username, command, user_comment FROM
> mysql.backup_history
> WHERE backup_id=@bup_id;
> -operation num_objects username command
> -backup 1 root BACKUP DATABASE backup_logs TO 'backup_logs1.bak'
> +operation num_objects username command user_comment
> +backup 1 root BACKUP DATABASE backup_logs TO 'backup_logs1.bak'
> Include all objects in database(Databases, tables, procedures and
> functions, views, triggers and events) and perform backup operation.
> con1: Create tables
> @@ -397,6 +397,63 @@ SET @@global.backup_history_log_file = r
> ERROR HY000: The path specified for backup_history_log_file is too long.
> SET @@global.backup_history_log_file = DEFAULT;
> SET global max_allowed_packet=DEFAULT;
> +# Check normal length comment string.
> +# The backup id for this command should be 506.
> +BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT
> +"This is a really long comment. Much longer than expected.";
> +backup_id
> +506
> +# Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> +WHERE backup_id=506;
> +operation num_objects username command user_comment
> +backup 11 tom BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT
> +"This is a really long comment. Much longer than expected."
> +# Check too long of a comment string.
> +# Should truncate with 3 Y's.
> +# The backup id for this command should be 507.
> +BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT
> +"This is a really long comment xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxYYYYYYxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
> +backup_id
> +507
> +# Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> +WHERE backup_id=507;
> +operation num_objects username command user_comment
> +backup 11 tom BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT
> +"This is a really long comment xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxYYY
> +# Check zero length comment string.
> +# The backup id for this command should be 508.
> +RESTORE FROM 'backup_logs_orig.bak' OVERWRITE COMMENT "This is a restore with
> backup_id = 508.";
> +backup_id
> +508
> +# Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> +WHERE backup_id=508;
> +operation num_objects username command user_comment
> +restore 11 tom RESTORE FROM 'backup_logs_orig.bak' OVERWRITE COMMENT "This is a
> restore with backup_id = 508."
> +# Check zero length comment string.
> +# The backup id for this command should be 509.
> +BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT "";
> +backup_id
> +509
> +# Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> +WHERE backup_id=509;
> +operation num_objects username command user_comment
> +backup 11 tom BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT ""
> SET @@global.log_backup_output = 'TABLE';
> DROP USER 'tom'@'localhost';
> SET DEBUG_SYNC= 'reset';
>
> === modified file 'mysql-test/suite/backup/t/backup_logs.test'
> --- a/mysql-test/suite/backup/t/backup_logs.test 2009-03-09 14:00:03 +0000
> +++ b/mysql-test/suite/backup/t/backup_logs.test 2009-03-15 19:57:33 +0000
> @@ -112,7 +112,7 @@ SET DEBUG_SYNC= 'after_backup_log_init
> SET DEBUG_SYNC= 'after_backup_start_backup SIGNAL running WAIT_FOR finish';
>
> --echo Perform backup database operation with database alone.
> -send BACKUP DATABASE backup_logs TO 'backup_logs1.bak';
> +send BACKUP DATABASE backup_logs TO 'backup_logs1.bak' COMMENT "Test 2";
>
> connection default;
>
> @@ -132,7 +132,7 @@ reap;
> --echo Get last backup_id
> SELECT MAX(backup_id) INTO @bup_id FROM mysql.backup_history
> WHERE command LIKE "BACKUP DATABASE backup_logs TO%";
> -SELECT operation,num_objects, username, command FROM mysql.backup_history
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> WHERE backup_id=@bup_id;
> --remove_file $MYSQLD_BACKUPDIR/backup_logs1.bak
>
> @@ -145,7 +145,7 @@ BACKUP DATABASE backup_logs TO 'backup_l
> --echo Get last backup_id
> SELECT MAX(backup_id) INTO @bup_id FROM mysql.backup_history
> WHERE command LIKE "BACKUP DATABASE backup_logs TO%";
> -SELECT operation,num_objects, username, command FROM mysql.backup_history
> +SELECT operation,num_objects, username, command, user_comment FROM
> mysql.backup_history
> WHERE backup_id=@bup_id;
> --remove_file $MYSQLD_BACKUPDIR/backup_logs1.bak
>
> @@ -517,6 +517,51 @@ SET @@global.backup_history_log_file = D
> SET global max_allowed_packet=DEFAULT;
>
> #
> +# Check maximum and minimum length of user_comment field.
> +#
> +
> +--remove_file $MYSQLD_BACKUPDIR/backup_logs_orig.bak
> +--echo # Check normal length comment string.
> +--echo # The backup id for this command should be 506.
> +BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT
> +"This is a really long comment. Much longer than expected.";
> +--echo # Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> + WHERE backup_id=506;
> +
> +--remove_file $MYSQLD_BACKUPDIR/backup_logs_orig.bak
> +--echo # Check too long of a comment string.
> +--echo # Should truncate with 3 Y's.
> +--echo # The backup id for this command should be 507.
> +BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT
> +"This is a really long comment xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxYYYYYYxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
> +--echo # Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> + WHERE backup_id=507;
> +
> +--echo # Check zero length comment string.
> +--echo # The backup id for this command should be 508.
> +RESTORE FROM 'backup_logs_orig.bak' OVERWRITE COMMENT "This is a restore with
> backup_id = 508.";
> +--echo # Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> + WHERE backup_id=508;
> +
> +--remove_file $MYSQLD_BACKUPDIR/backup_logs_orig.bak
> +--echo # Check zero length comment string.
> +--echo # The backup id for this command should be 509.
> +BACKUP DATABASE backup_logs to 'backup_logs_orig.bak' COMMENT "";
> +--echo # Show data in backup_history log.
> +SELECT operation, num_objects, username, command, user_comment FROM
> mysql.backup_history
> + WHERE backup_id=509;
> +
> +#
> # Cleanup.
> #
>
>
> === modified file 'sql/backup/backup_kernel.h'
> --- a/sql/backup/backup_kernel.h 2009-02-20 16:40:19 +0000
> +++ b/sql/backup/backup_kernel.h 2009-03-15 19:57:33 +0000
> @@ -74,10 +74,12 @@ public:
>
> Backup_info* prepare_for_backup(String *location,
> LEX_STRING orig_loc,
> - const char*, bool);
> + const char*, bool,
> + const char*);
> Restore_info* prepare_for_restore(String *location,
> LEX_STRING orig_loc,
> - const char*, bool);
> + const char*, bool,
> + const char*);
>
> int do_backup();
> int do_restore(bool overwrite);
>
> === modified file 'sql/backup/kernel.cc'
> --- a/sql/backup/kernel.cc 2009-03-10 18:19:41 +0000
> +++ b/sql/backup/kernel.cc 2009-03-15 19:57:33 +0000
> @@ -172,7 +172,8 @@ execute_backup_command(THD *thd,
>
> Backup_info *info= context.prepare_for_backup(backupdir, lex->backup_dir,
> thd->query,
> - lex->backup_compression);
> + lex->backup_compression,
> + lex->comment.str);
> // reports errors
>
> if (!info || !info->is_valid())
> @@ -215,7 +216,8 @@ execute_backup_command(THD *thd,
> {
>
> Restore_info *info= context.prepare_for_restore(backupdir, lex->backup_dir,
> - thd->query, skip_gap_event);
> + thd->query, skip_gap_event,
> + lex->comment.str);
>
> if (!info || !info->is_valid())
> DBUG_RETURN(send_error(context, ER_BACKUP_RESTORE_PREPARE));
> @@ -579,6 +581,7 @@ int Backup_restore_ctx::prepare(::String
> @param[in] orig_loc path as specified on command line for backup image
> @param[in] query BACKUP query starting the operation
> @param[in] with_compression backup image compression switch
> + @param[in] comment comment specified on command line
>
> @returns Pointer to a @c Backup_info instance which can be used for selecting
> which objects to backup. NULL if an error was detected.
> @@ -594,7 +597,8 @@ Backup_info*
> Backup_restore_ctx::prepare_for_backup(String *backupdir,
> LEX_STRING orig_loc,
> const char *query,
> - bool with_compression)
> + bool with_compression,
> + const char *comment)
> {
> using namespace backup;
>
> @@ -614,6 +618,7 @@ Backup_restore_ctx::prepare_for_backup(S
>
> time_t when= my_time(0);
> report_start(when);
> + report_comment(comment);
>
> /*
> Do preparations common to backup and restore operations. After call
> @@ -692,6 +697,7 @@ Backup_restore_ctx::prepare_for_backup(S
> @param[in] orig_loc path as specified on command line for backup image
> @param[in] query RESTORE query starting the operation
> @param[in] skip_gap_event TRUE means do not write gap event
> + @param[in] comment comment specified on command line
>
> @returns Pointer to a @c Restore_info instance containing catalogue of the
> backup image (read from the image). NULL if errors were detected.
> @@ -702,7 +708,8 @@ Restore_info*
> Backup_restore_ctx::prepare_for_restore(String *backupdir,
> LEX_STRING orig_loc,
> const char *query,
> - bool skip_gap_event)
> + bool skip_gap_event,
> + const char *comment)
> {
> using namespace backup;
>
> @@ -737,6 +744,7 @@ Backup_restore_ctx::prepare_for_restore(
>
> time_t when= my_time(0);
> report_start(when);
> + report_comment(comment);
>
> /*
> Do preparations common to backup and restore operations. After this call
>
> === modified file 'sql/backup/logger.h'
> --- a/sql/backup/logger.h 2009-02-13 13:25:43 +0000
> +++ b/sql/backup/logger.h 2009-03-15 19:57:33 +0000
> @@ -67,6 +67,7 @@ public:
> void report_master_binlog_pos(const st_bstream_binlog_pos&);
> void report_driver(const char *driver);
> void report_backup_file(char * path);
> + void report_comment(const char * str);
> void report_stats_pre(const Image_info&);
> void report_stats_post(const Image_info&);
>
> @@ -349,6 +350,19 @@ void Logger::report_backup_file(char *pa
> backup_log->backup_file(path);
> }
>
> +/**
> + Report comment from user.
> +*/
> +inline
> +void Logger::report_comment(const char *str)
> +{
> + DBUG_ASSERT(m_state == RUNNING);
> + if (!str)
> + backup_log->comment(str);
> + else
> + backup_log->comment(0);
> +}
> +
> /**
> Initialize logger for backup or restore operation.
>
>
> === modified file 'sql/si_logs.cc'
> --- a/sql/si_logs.cc 2008-10-30 17:53:24 +0000
> +++ b/sql/si_logs.cc 2009-03-15 19:57:33 +0000
> @@ -88,6 +88,21 @@ void Backup_log::backup_file(const char
> }
>
> /**
> + Report the comment entered on the command line.
> +
> + This method updates the comment information in the history data.
> +
> + @param[IN] char * str The comment from the command line.
> +*/
> +void Backup_log::comment(const char *str)
> +{
> + if (!str)
> + m_op_hist.comment= (char *)str;
> + else
> + m_op_hist.comment= 0;
> +}
> +
> +/**
> Write history data.
>
> This method calls the server's logger to write the backup_history log
>
> === modified file 'sql/si_logs.h'
> --- a/sql/si_logs.h 2008-11-14 14:49:09 +0000
> +++ b/sql/si_logs.h 2009-03-15 19:57:33 +0000
> @@ -60,6 +60,7 @@ struct st_backup_history
> String driver_name; ///< list of backup engines used
> int master_binlog_pos; ///< position in the binary log
> char *master_binlog_file; ///< name of the master's binary log file
> + char *comment; ///< comment from user on command line
> };
>
>
> @@ -146,6 +147,7 @@ public:
> void vp_time(time_t when, bool report);
> void add_driver(const char* driver);
> void backup_file(const char *full_path);
> + void comment(const char* str);
>
> private:
> st_backup_history m_op_hist; ///< history log information
>
> === modified file 'sql/sql_yacc.yy'
> --- a/sql/sql_yacc.yy 2009-03-09 14:00:03 +0000
> +++ b/sql/sql_yacc.yy 2009-03-15 19:57:33 +0000
> @@ -1272,7 +1272,7 @@ bool my_yyoverflow(short **a, YYSTYPE **
> opt_natural_language_mode opt_query_expansion
> opt_ev_status opt_ev_on_completion ev_on_completion opt_ev_comment
> ev_alter_on_schedule_completion opt_ev_rename_to opt_ev_sql_stmt
> - opt_transactional_lock_timeout
> + opt_transactional_lock_timeout opt_backup_comment
> /* opt_lock_timeout_value */
>
> %type <m_fk_option>
> @@ -6644,7 +6644,7 @@ restore:
> }
> FROM
> TEXT_STRING_sys
> - opt_overwrite opt_skip_gap_event
> + opt_overwrite opt_skip_gap_event opt_backup_comment
> {
> Lex->backup_dir = $4;
> }
> @@ -6702,7 +6702,7 @@ backup:
> database_list
> TO_SYM
> TEXT_STRING_sys
> - opt_compression
> + opt_compression opt_backup_comment
> {
> Lex->backup_dir = $6;
> }
> @@ -6735,6 +6735,15 @@ opt_compression_algorithm:
> }
> ;
>
> +opt_backup_comment:
> + /* empty */ { $$= 0; }
> + | COMMENT_SYM TEXT_STRING_sys
> + {
> + Lex->comment= $2;
> + $$= 1;
> + }
> + ;
> +
> database_list:
> '*'
> {}
>
>
