Chuck,
I had a quick look at your preliminary patch. All looks ok except for one small
thing - see below.
cbell@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 6.0 repository of cbell. When cbell 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, 2008-01-23 17:51:19-05:00, cbell@mysql_cab_desk. +4 -0
> BUG#33355 : Backup: hang if database is mysql
>
> Code now warns user if mysql or information_schema is found in
> the database list. Code throws error if database list is empty.
>
> mysql-test/r/backup_errors.result@stripped, 2008-01-23 17:51:11-05:00,
> cbell@mysql_cab_desk. +21 -0
> BUG#33355 : Backup: hang if database is mysql
>
> New result file.
>
> mysql-test/t/backup_errors.test@stripped, 2008-01-23 17:51:12-05:00,
> cbell@mysql_cab_desk. +66 -0
> BUG#33355 : Backup: hang if database is mysql
>
> Updated test.
>
> sql/backup/kernel.cc@stripped, 2008-01-23 17:51:12-05:00, cbell@mysql_cab_desk. +19 -2
> BUG#33355 : Backup: hang if database is mysql
>
> Added code to send error if no databases to backup.
> Added code to capture and warn if mysql or information_schema
> included in database list.
>
> sql/share/errmsg.txt@stripped, 2008-01-23 17:51:12-05:00, cbell@mysql_cab_desk. +4 -0
> BUG#33355 : Backup: hang if database is mysql
>
> New error/warning messages.
>
> diff -Nrup a/mysql-test/r/backup_errors.result b/mysql-test/r/backup_errors.result
> --- a/mysql-test/r/backup_errors.result 2007-12-13 11:52:49 -05:00
> +++ b/mysql-test/r/backup_errors.result 2008-01-23 17:51:11 -05:00
> @@ -22,3 +22,24 @@ BACKUP DATABASE test,foo,bdb,bar TO 'tes
> ERROR 42000: Unknown database 'foo,bar'
> DROP DATABASE adb;
> DROP DATABASE bdb;
> +Backup of mysql, information_schema scenario 1
> +BACKUP DATABASE mysql TO 't.bak';
> +ERROR HY000: Nothing to backup
> +Backup of mysql, information_schema scenario 2
> +BACKUP DATABASE information_schema TO 't.bak';
> +ERROR HY000: Nothing to backup
> +Backup of mysql, information_schema scenario 3
> +BACKUP DATABASE mysql, information_schema TO 't.bak';
> +ERROR HY000: Nothing to backup
> +Backup of mysql, information_schema scenario 4
> +BACKUP DATABASE mysql, test TO 't.bak';
> +backup_id
> +#
> +Backup of mysql, information_schema scenario 5
> +BACKUP DATABASE information_schema, test TO 't.bak';
> +backup_id
> +#
> +Backup of mysql, information_schema scenario 6
> +BACKUP DATABASE mysql, information_schema, test TO 't.bak';
> +backup_id
> +#
> diff -Nrup a/mysql-test/t/backup_errors.test b/mysql-test/t/backup_errors.test
> --- a/mysql-test/t/backup_errors.test 2007-12-13 11:58:31 -05:00
> +++ b/mysql-test/t/backup_errors.test 2008-01-23 17:51:12 -05:00
> @@ -57,3 +57,69 @@ DROP DATABASE bdb;
> # will fail and we will be warned that something is wrong with the test
> --error 1
> --remove_file $MYSQLTEST_VARDIR/master-data/test.bak
> +
> +# Check error conditions for including mysql and information_schema in
> +# the list of databases.
> +#
> +# There are several scenarios:
> +# 1) BACKUP DATABASE mysql TO 't.bak'
> +# Error: Nothing to backup
> +# 2) BACKUP DATABASE information_schema TO 't.bak'
> +# Error: Nothing to backup
> +# 3) BACKUP DATABASE mysql, information_schema TO 't.bak'
> +# Error: Nothing to backup
> +# 4) BACKUP DATABASE mysql, test TO 't.bak'
> +# Warning: Backup cannot include database 'mysql'
> +# 5) BACKUP DATABASE information_schema, test TO 't.bak'
> +# Warning: Backup cannot include database 'information_schema'
> +# 6) BACKUP DATABASE mysql, information_schema, test TO 't.bak'
> +# Warning: Backup cannot include database 'mysql'
> +# Warning: Backup cannot include database 'mysql'
> +
> +# Scenario 1
> +--echo Backup of mysql, information_schema scenario 1
> +--error ER_BACKUP_NOTHING_TO_BACKUP
> +BACKUP DATABASE mysql TO 't.bak';
> +
> +--error 0, 1
> +--remove_file $MYSQLTEST_VARDIR/master-data/t.bak
> +
> +# Scenario 2
> +--echo Backup of mysql, information_schema scenario 2
> +--error ER_BACKUP_NOTHING_TO_BACKUP
> +BACKUP DATABASE information_schema TO 't.bak';
> +
> +--error 0, 1
> +--remove_file $MYSQLTEST_VARDIR/master-data/t.bak
> +
> +# Scenario 3
> +--echo Backup of mysql, information_schema scenario 3
> +--error ER_BACKUP_NOTHING_TO_BACKUP
> +BACKUP DATABASE mysql, information_schema TO 't.bak';
> +
> +--error 0, 1
> +--remove_file $MYSQLTEST_VARDIR/master-data/t.bak
> +
> +# Scenario 4
> +--echo Backup of mysql, information_schema scenario 4
> +--replace_column 1 #
> +BACKUP DATABASE mysql, test TO 't.bak';
> +
> +--error 0, 1
> +--remove_file $MYSQLTEST_VARDIR/master-data/t.bak
> +
> +# Scenario 5
> +--echo Backup of mysql, information_schema scenario 5
> +--replace_column 1 #
> +BACKUP DATABASE information_schema, test TO 't.bak';
> +
> +--error 0, 1
> +--remove_file $MYSQLTEST_VARDIR/master-data/t.bak
> +
> +# Scenario 6
> +--echo Backup of mysql, information_schema scenario 6
> +--replace_column 1 #
> +BACKUP DATABASE mysql, information_schema, test TO 't.bak';
> +
> +--error 0, 1
> +--remove_file $MYSQLTEST_VARDIR/master-data/t.bak
> diff -Nrup a/sql/backup/kernel.cc b/sql/backup/kernel.cc
> --- a/sql/backup/kernel.cc 2007-12-14 02:51:28 -05:00
> +++ b/sql/backup/kernel.cc 2008-01-23 17:51:12 -05:00
> @@ -295,7 +295,16 @@ execute_backup_command(THD *thd, LEX *le
> else
> {
> info.write_message(log_level::INFO,"Backing up selected databases");
> - info.add_dbs(lex->db_list); // backup databases specified by user
> + res= info.add_dbs(lex->db_list); // backup databases specified by user
> + }
> + if ((info.db_count() == 0) && (res != ERROR))
> + {
> + res= ERROR;
> + info.report_error(log_level::ERROR, ER_BACKUP_NOTHING_TO_BACKUP, MYF(0));
> + stop= my_time(0);
> + info.save_end_time(stop);
> + report_errors(thd, info, ER_BACKUP_BACKUP);
> + goto backup_error;
> }
>
> report_ob_num_objects(backup_prog_id, info.table_count);
> @@ -697,7 +706,15 @@ int Backup_info::add_dbs(List< ::LEX_STR
> String db_name(*s);
> Db_ref db(db_name);
>
> - if (db_exists(db))
> + if (((my_strcasecmp(system_charset_info,
> + db_name.c_ptr(), "information_schema") == 0)) ||
> + (my_strcasecmp(system_charset_info,
> + db_name.c_ptr(), "mysql") == 0))
> + {
> + report_error(log_level::WARNING, ER_BACKUP_CANNOT_INCLUDE_DB,
> + db_name.c_ptr());
> + }
I think the loop should be broken with "goto error" here.
> + else if (db_exists(db))
> {
> if (!unknown_dbs.is_empty()) // we just compose unknown_dbs list
> continue;
> diff -Nrup a/sql/share/errmsg.txt b/sql/share/errmsg.txt
> --- a/sql/share/errmsg.txt 2007-12-13 11:49:58 -05:00
> +++ b/sql/share/errmsg.txt 2008-01-23 17:51:12 -05:00
> @@ -6114,6 +6114,10 @@ ER_BACKUP_RESTORE_START
> ER_BACKUP_RESTORE_DONE
> eng "Restore completed"
>
> +ER_BACKUP_NOTHING_TO_BACKUP
> + eng "Nothing to backup"
> +ER_BACKUP_CANNOT_INCLUDE_DB
> + eng "Database '%-.64s' cannot be included in a backup and will be skipped"
> ER_BACKUP_BACKUP
> eng "Error during backup operation - server's error log contains more
> information about the error"
> ER_BACKUP_RESTORE
>
>
Rafal
