From: Konstantin Osipov Date: October 24 2009 9:09am Subject: Re: bzr commit into mysql-6.0-codebase-bugfixing branch (lars-erik.bjork:3655) Bug#48067 List-Archive: http://lists.mysql.com/commits/88076 Message-Id: <20091024090902.GA6537@shakti> MIME-Version: 1.0 Content-Type: text/plain; CHARSET=US-ASCII Content-Transfer-Encoding: 7BIT * Lars-Erik.Bjork@stripped [09/10/15 23:49]: > 3655 lars-erik.bjork@stripped 2009-10-15 > This is a patch for bug#48067 > "A temp table with the same name as an existing table, makes drop > database fail" > > When dropping the database, mysql_rm_known_files() reads the contents > of the database directory, and creates a TABLE_LIST object, for each > .frm file encountered. Temporary tables, however, are not associated > with any .frm file. > > The list of tables to drop are passed to mysql_rm_table_part2(). > This method prefers temporary tables over regular tables, so if > there is a temporary table with the same name as a regular, the > temporary is removed, leaving the regular table intact. > Regular tables are only deleted if there are no temporary tables > with the same name. > > This fix ensures, that for all TABLE_LIST objects that are created > by mysql_rm_known_files(), 'open_type' is set to 'OT_BASE_ONLY', to > indicate that this is a regular table. In all cases in > mysql_rm_table_part2() where we prefer a temporary table to a > non-temporary table, we chek if 'open_type' equals 'OT_BASE_ONLY'. This is a very good comment, but I would rephrase this last sentence a bit: In all cases in mysql_rm_table_part2() where we prefer a temporary table to a non-temporary table, we check that open_type is not set to 'OT_BASE_ONLY'. In this latter case non-temporary tables always take precedence. > @ mysql-test/r/temp_table.result > The expected result of the test. > @ mysql-test/t/temp_table.test > Test based on the bug report. IMHO this changeset comment is useless ;) You could mention bug id, it would make it a bit useful. > @ sql/sql_db.cc > For all TABLE_LIST objects that are created by mysql_rm_known_files(), > 'open_type' is set to 'OT_BASE_ONLY', to indicate that this is a > regular table. > @ sql/sql_table.cc > Check if 'open_type' is set to 'OT_BASE_ONLY, every place a temporary table is > preferred to a non-temporary table. > > modified: > mysql-test/r/temp_table.result > mysql-test/t/temp_table.test > sql/sql_db.cc > sql/sql_table.cc > === modified file 'mysql-test/r/temp_table.result' > --- a/mysql-test/r/temp_table.result 2009-01-07 12:11:37 +0000 > +++ b/mysql-test/r/temp_table.result 2009-10-15 19:34:27 +0000 > @@ -210,4 +210,16 @@ UPDATE t1,t2 SET t1.a = t2.a; > INSERT INTO t2 SELECT f1(); > DROP TABLE t1,t2,t3; > DROP FUNCTION f1; > +# > +# Bug #48067: A temp table with the same name as an existing table, > +# makes drop database fail. > +# > +DROP TEMPORARY TABLE IF EXISTS bug48067.t1; > +DROP DATABASE IF EXISTS bug48067; > +CREATE DATABASE bug48067; > +CREATE TABLE bug48067.t1 (c1 int); > +INSERT INTO bug48067.t1 values (1); > +CREATE TEMPORARY TABLE bug48067.t1 (c1 int); > +DROP DATABASE bug48067; > +DROP TEMPORARY table bug48067.t1; > End of 5.1 tests OK, but please add a test case for the other branch in mysql_rm_table_part2 that you modified -- DROP DATABASE in LOCK TABLES mode. > === modified file 'mysql-test/t/temp_table.test' > --- a/mysql-test/t/temp_table.test 2009-01-07 12:11:37 +0000 > +++ b/mysql-test/t/temp_table.test 2009-10-15 19:34:27 +0000 > @@ -235,4 +235,19 @@ INSERT INTO t2 SELECT f1(); > DROP TABLE t1,t2,t3; > DROP FUNCTION f1; > > +--echo # > +--echo # Bug #48067: A temp table with the same name as an existing table, > +--echo # makes drop database fail. > +--echo # > +--disable_warnings > +DROP TEMPORARY TABLE IF EXISTS bug48067.t1; > +DROP DATABASE IF EXISTS bug48067; > +--enable_warnings > +CREATE DATABASE bug48067; > +CREATE TABLE bug48067.t1 (c1 int); > +INSERT INTO bug48067.t1 values (1); > +CREATE TEMPORARY TABLE bug48067.t1 (c1 int); > +DROP DATABASE bug48067; > +DROP TEMPORARY table bug48067.t1; > + > --echo End of 5.1 tests > > === modified file 'sql/sql_db.cc' > --- a/sql/sql_db.cc 2009-10-09 09:53:31 +0000 > +++ b/sql/sql_db.cc 2009-10-15 19:34:27 +0000 > @@ -1152,6 +1152,7 @@ static long mysql_rm_known_files(THD *th > (void) filename_to_tablename(file->name, table_list->table_name, > MYSQL50_TABLE_NAME_PREFIX_LENGTH + > strlen(file->name) + 1); > + table_list->open_type= OT_BASE_ONLY; > > /* To be able to correctly look up the table in the table cache. */ > if (lower_case_table_names) > > === modified file 'sql/sql_table.cc' > --- a/sql/sql_table.cc 2009-09-29 09:36:22 +0000 > +++ b/sql/sql_table.cc 2009-10-15 19:34:27 +0000 > @@ -1906,7 +1906,8 @@ int mysql_rm_table_part2(THD *thd, TABLE > else > { > for (table= tables; table; table= table->next_local) > - if (find_temporary_table(thd, table->db, table->table_name)) > + if (table->open_type != OT_BASE_ONLY && > + find_temporary_table(thd, table->db, table->table_name)) Please don't use tabs (at all) in the server code. > { > /* > A temporary table. > @@ -1951,8 +1952,11 @@ int mysql_rm_table_part2(THD *thd, TABLE > table->db, table->table_name, table->table, > table->table ? table->table->s : (TABLE_SHARE *)-1)); > > - error= drop_temporary_table(thd, table); > - > + if (table->open_type == OT_BASE_ONLY) > + error= 1; > + else > + error= drop_temporary_table(thd, table); > + > switch (error) { > case 0: > // removed temporary table The patch is OK to push after fixing the comment, the alignment and extending the test. Thank you for working on this! --