← Back to team overview

maria-developers team mailing list archive

Re: 65b0617: MDEV-10846 Running mysqldump backup twice returns error: Table

 

Hi, Alexey!

Ok to push, thanks!
Just fix the test first (see below)

On Oct 22, Alexey Botchkov wrote:
> revision-id: 65b0617836390bb76104cb37094e16bafa6da8b2 (mariadb-10.0.27-12-g65b0617)
> parent(s): fb38d2642011c574cc9103ae1a1f9dd77f7f027e
> committer: Alexey Botchkov
> timestamp: 2016-10-22 12:08:15 +0400
> message:
> 
> MDEV-10846 Running mysqldump backup twice returns error: Table
> 'mysql.proc' doesn't exist.
> 
>         The mysql_rm_db() doesn't seem to expect the 'mysql' database
>         to be deleted. Checks for that added.
>         Also fixed the bug MDEV-11105 Table named 'db' has weird side effect.
>         The db.opt file now removed separately.
> 
> ---
>  mysql-test/r/drop.result |  6 ++++++
>  mysql-test/t/drop.test   |  9 +++++++++
>  sql/sql_db.cc            | 26 +++++++++++++++++++++-----
>  3 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/mysql-test/r/drop.result b/mysql-test/r/drop.result
> index c23ffbe3..ee1758f 100644
> --- a/mysql-test/r/drop.result
> +++ b/mysql-test/r/drop.result
> @@ -209,3 +209,9 @@ INSERT INTO table1 VALUES (1);
>  ERROR 42S02: Unknown table 't.notable'
>  DROP TABLE table1,table2;
>  # End BUG#34750
> +#
> +# MDEV-11105 Table named 'db' has weird side effect.
> +#
> +CREATE DATABASE mysqltest;
> +CREATE TABLE mysqltest.t1(id INT);

huh? the table name is supposed to be 'db' here :)

> +DROP DATABASE mysqltest;
> diff --git a/sql/sql_db.cc b/sql/sql_db.cc
> index e89c3d9..0a3ff64 100644
> --- a/sql/sql_db.cc
> +++ b/sql/sql_db.cc
> @@ -784,7 +784,7 @@ bool mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create_info)
>  bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent)
>  {
>    ulong deleted_tables= 0;
> -  bool error= true;
> +  bool error= true, rm_mysql_schema;
>    char	path[FN_REFLEN + 16];
>    MY_DIR *dirp;
>    uint length;
> @@ -809,6 +809,18 @@ bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent)
>    length= build_table_filename(path, sizeof(path) - 1, db, "", "", 0);
>    strmov(path+length, MY_DB_OPT_FILE);		// Append db option file name
>    del_dbopt(path);				// Remove dboption hash entry
> +  /*
> +     Now remove the db.opt file.
> +     The 'find_db_tables_and_rm_known_files' doesn't remove this file
> +     if there exists a table with the name 'db', so let's just do it
> +     separately. We know this file exists and needs to be deleted anyway.

ah, thanks, I see now. I wondered why a table 'db' would matter.

> +  */
> +  if (my_delete_with_symlink(path, MYF(0)) && my_errno != ENOENT)
> +  {
> +    my_error(EE_DELETE, MYF(0), path, my_errno);
> +    DBUG_RETURN(true);

here I don't think this error is quite correct. nowhere else
mysql_rm_db() seems to fail with EE_DELETE. It always says
"Error dropping database (can't rmdir './exp_db', errno: 39 "Directory not empty")"
May be you should add this EE_DELETE as a warning?

> +  }
> +    
>    path[length]= '\0';				// Remove file name

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx