STATUS
------
Not (yet) approved - one request made.
REQUESTS
--------
1. Increase size of newname buffer to satisfy my_uuid2str requirements [2].
SUGGESTIONS
-----------
2. Change name of error constant to something which does not mention RESTORE [1].
DETAILS
-------
Ingo Struewing wrote:
> #At file:///home2/mydev/bzrroot/mysql-6.0-bug42572-2/ based on
> revid:charles.bell@stripped
>
> 2815 Ingo Struewing 2009-05-19
> Bug#42572 - Restore fails when myisampack used with -b option
>
> The use of myisampack -b creates a table backup file in the
> database directory. On DROP DATABASE, this file is not
> noticed as belonging to a database object. It is not deleted.
> DROP DATABASE fails because the directory is not empty.
>
> The same situation exists if the user creates files in the
> database directory, which are not known to MySQL as belonging
> to database objects. For example backup image files.
>
> RESTORE ignored the failing DROP DATABASE. It failed when a
> CREATE DATABASE for the not dropped database failed.
>
> The problem is solved by renaming the non-empty database
> directory, if DROP DATABASE fails due to unknown files.
> @ mysql-test/suite/backup/r/backup_external.result
> Bug#42572 - Restore fails when myisampack used with -b option
> Updated test result.
> @ mysql-test/suite/backup/t/backup_external.test
> Bug#42572 - Restore fails when myisampack used with -b option
> Fixed test.
> @ sql/share/errmsg.txt
> Bug#42572 - Restore fails when myisampack used with -b option
> Added ER_RESTORE_RENAME.
> @ sql/si_objects.cc
> Bug#42572 - Restore fails when myisampack used with -b option
> Added Abstract_obj::m_errno.
> Added Database_obj::drop()
>
> modified:
> mysql-test/suite/backup/r/backup_external.result
> mysql-test/suite/backup/t/backup_external.test
> sql/share/errmsg.txt
> sql/si_objects.cc
...
> === modified file 'sql/share/errmsg.txt'
> --- a/sql/share/errmsg.txt 2009-05-14 16:52:22 +0000
> +++ b/sql/share/errmsg.txt 2009-05-19 09:28:51 +0000
> @@ -6516,3 +6516,5 @@ ER_BACKUP_INTERRUPTED
> eng "Operation has been interrupted."
> ER_BACKUP_NOT_ENABLED
> eng "The MySQL Backup system is disabled in this release. Use --new on server
> startup to enable."
> +ER_RESTORE_RENAME
> + eng "Renamed directory with unknown files to '%.200s'"
>
[1] I suggest to change error constant name to e.g., ER_DB_DROP_RENAME. This is
to emphasize that si_objects is not a part of backup/restore system, but a
separate module in the server.
> === modified file 'sql/si_objects.cc'
> --- a/sql/si_objects.cc 2009-05-05 17:42:58 +0000
> +++ b/sql/si_objects.cc 2009-05-19 09:28:51 +0000
...
> +
> +bool Database_obj::drop(THD *thd)
> +{
> + bool rc;
> + DBUG_ENTER("Database_obj::drop");
> +
> + /*
> + If DROP DATABASE fails due to unknown files in the directory,
> + fix that by renaming the directory.
> + */
> + rc= Abstract_obj::drop(thd);
> + if (rc)
> + {
> + DBUG_PRINT("si_objects", ("drop database failed, m_errno: %d", m_errno));
> + if (m_errno == ER_DB_DROP_RMDIR)
> + {
> + size_t len= get_name()->length();
> + char oldname[FN_REFLEN];
> + char newname[FN_REFLEN];
[2] I think newname buffer should have size FN_REFLEN + MY_UUID_STRING_LENGTH +
1 to satisfy my_uuid2str() requirements. This is because oldname can take up to
FN_REFLEN bytes and then there should still be MY_UUID_STRING_LENGTH + 1 bytes
left for the uuid.
> + uchar uuid[MY_UUID_SIZE];
> +
> + DBUG_PRINT("si_objects", ("Renaming to oldname-uuid"));
> + /*
> + get_name() returns a const String. We cannot use c_ptr*() with it.
> + New name is ./dbname-UUID
> + */
> + if (len + 3 + MY_UUID_STRING_LENGTH >= FN_REFLEN)
> + goto err; /* purecov: inspected */
> + memcpy(oldname, get_name()->ptr(), len);
> + oldname[len] = '\0';
> + /* Convert database name to a file name. */
> + len= build_table_filename(oldname, sizeof(oldname)-1,
> + oldname, "", "", 0);
> + /* Remove trailing slash. */
> + if (len && (oldname[len-1] == FN_LIBCHAR))
> + oldname[--len]= '\0';
> + /* Build new name as old + uuid. */
> + memcpy(newname, oldname, len);
> + newname[len++]= '-';
> + my_uuid(uuid);
> + my_uuid2str(uuid, newname + len);
[3] Here, there should be at least MY_UUID_STRING_LENGTH + 1 bytes left at
newname + len.
Rafal
