Hi Jorgen,
STATUS
------
Changes requested.
REQUIREMENTS
------------
1. In tests, list databases after each restore to see their names [1,2].
2. Fix logic of get_database_stub() (see comments below).
SUGGESTIONS
-----------
2. Remove "delete obj" when obj is NULL [3].
COMMENTS
--------
I think that the change I proposed, unfortunately, is not correct. This is
because function obs::get_database_stub() is specified to return obs::Obj
instance even for databases which do not exist in the server at the moment. With
the changes you implemented it will return NULL if named database does not exist
- this violates the specification.
I would still try to go with this solution and ensure that the name stored in
obs::Obj is always normalized. To satisfy the specification, the logic of
get_database_stub() could be changed to this:
if (lctn == 0)
<return Obj with exact name requested>;
if (lctn == 1)
<convert requested name to lowercase>;
<execute select on I_S.SCHEMATA (ignoring case)>;
if (<a row was found>)
<return Obj with name found by the SELECT>;
else
<return object with given name (which is translated to lc in case lctn==1)>;
This way, even if lctn==2, a call to get_database_stub('Xx') will return object
with correct name, if a database with name 'XX' or 'Xx' or 'xX' or 'xx' exists
in the server, otherwise it will return object with given name 'Xx'.
Note also that a NULL return from get_database_stub() indicates an error (not
that the database does not exists) and the logic in add_dbs() should take it
into account (as the original logic does).
DETAILS
-------
> 2784 Jorgen Loland 2009-03-05
> Bug#39063 - Online Backup: Backup behavior changes with the case used for the
> database name
>
> Before, the database list for BACKUP was case sensitive. This caused problems
> in case insensitive servers because database 'X' and 'x' is considered the same database
> while backup would fail if the wrong case was used.
>
> With this patch, the database names are converted to lower case when BACKUP is
> executed on a case insensitive server.
...
> === added file 'mysql-test/suite/backup/t/backup_dbname.test'
...
> +--echo #
> +--echo # RESTORE databases with correct case
> +--echo #
> +--echo # RESTORE lower case database and verify content
> +--echo #
> +--replace_column 1 #
> +RESTORE FROM 'lowlow.bup';
> +
> +--echo
[1] Add "SHOW DATABASES (LIKE ...)" or similar to see the names of restored
database(s). Do it after each RESTORE.
...
> === added file 'mysql-test/suite/backup/t/backup_dbname_notwin.test'
...
> +--echo #
> +--echo # RESTORE lower case database and verify content
> +--echo #
> +--replace_column 1 #
> +RESTORE FROM 'lcase.bup';
> +
> +--echo
[2] Add "SHOW DATABASES (LIKE ...)" or similar to see the names of restored
database(s).
> === modified file 'sql/backup/backup_info.cc'
> --- a/sql/backup/backup_info.cc 2009-02-23 21:32:11 +0000
> +++ b/sql/backup/backup_info.cc 2009-03-05 11:49:15 +0000
> @@ -642,47 +642,53 @@ int Backup_info::add_dbs(THD *thd, List<
> {
> backup::String db_name(*s);
>
> - // Ignore the database if it has already been inserted into the catalogue.
> - if (has_db(db_name))
> - continue;
> -
> if (is_internal_db_name(&db_name))
> {
> m_log.report_error(ER_BACKUP_CANNOT_INCLUDE_DB, db_name.c_ptr());
> goto error;
> }
>
> - obs::Obj *obj= get_database_stub(&db_name); // reports errors
> -
> - if (obj && !check_db_existence(thd, &db_name))
> - {
> - if (!unknown_dbs.is_empty()) // we just compose unknown_dbs list
> - {
> - delete obj;
> - continue;
> - }
> + obs::Obj *obj= get_database_stub(thd, &db_name); // reports errors
> +
> + if (!obj)
> + {
> + if (!unknown_dbs.is_empty())
> + unknown_dbs.append(",");
> + unknown_dbs.append(db_name);
>
> - Db *db= add_db(obj); // reports errors
> + delete obj;
[3] No need to delete NULL pointer.
Rafal
