From: Rafal Somla Date: October 29 2008 10:57am Subject: bzr commit into mysql-6.0-backup branch (Rafal.Somla:2718) Bug#33574 Bug#34903 List-Archive: http://lists.mysql.com/commits/57289 X-Bug: 33574,34903 Message-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #At file:///ext/mysql/bzr/backup/bug33574/ 2718 Rafal Somla 2008-10-29 BUG#33574, BUG#34903 (Backup: restore failure if temporary table exists) Before: If a temporary table existed with the same name as a regular one, BACKUP saved the temporary table. After: Temporary tables are ignored by BACKUP command. added: mysql-test/suite/backup_engines/r/backup_tmp_tables.result mysql-test/suite/backup_engines/t/backup_tmp_tables.test modified: sql/backup/be_snapshot.cc sql/backup/be_thread.cc sql/backup/kernel.cc sql/si_objects.cc per-file messages: mysql-test/suite/backup_engines/t/backup_tmp_tables.test Test checking that tmp tables do not occlude tables being backed up. sql/backup/be_snapshot.cc Pass MYSQL_OPEN_SKIP_TEMPORARY flag when openning and locking tables. sql/backup/be_thread.cc Pass MYSQL_OPEN_SKIP_TEMPORARY flag when opening and locking tables. sql/backup/kernel.cc Pass MYSQL_OPEN_SKIP_TEMPORARY flag when lcoking tables for restore operation. sql/si_objects.cc Pass MYSQL_OPEN_SKIP_TEMPORARY flag when openning a table to get its CREATE statement. === added file 'mysql-test/suite/backup_engines/r/backup_tmp_tables.result' --- a/mysql-test/suite/backup_engines/r/backup_tmp_tables.result 1970-01-01 00:00:00 +0000 +++ b/mysql-test/suite/backup_engines/r/backup_tmp_tables.result 2008-10-29 10:56:16 +0000 @@ -0,0 +1,58 @@ +SHOW VARIABLES LIKE 'storage_engine'; +Variable_name Value +storage_engine # +** Pre-cleanup +DROP DATABASE IF EXISTS db; +** Create a database +CREATE DATABASE db; +USE db; +** Create regular tables +CREATE TABLE t1 (a int); +CREATE TABLE t2 (a char(1)); +** Create a view +CREATE VIEW v1 AS SELECT * FROM t1; +** Store table's definition for later check +** Insert some data into the tables +INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES ('x'); +** Create temporary tables with the same name, but different layout +** and using different storage engines +CREATE TEMPORARY TABLE t1 (b text, c int) ENGINE=MyISAM; +CREATE TEMPORARY TABLE t2 (b int, c blob) ENGINE=InnoDB; +** Insert data into the temporary tables +INSERT INTO t1 VALUES ('foo', 2); +INSERT INTO t2 VALUES (3, 'bar'); +** Backup database +BACKUP DATABASE db TO 'db.bkp'; +backup_id +# +** Drop and restore the database +DROP TABLE t1; +DROP TABLE t2; +RESTORE FROM 'db.bkp'; +backup_id +# +** Check definitions of the tables after restore +table_t1 +0 +table_t2 +0 +** Checking data after restore +SELECT * FROM t1; +a +1 +SELECT * FROM t2; +a +x +SELECT * FROM v1; +a +1 +** Checking if restored table is seen from other connection +SELECT * FROM db.t1; +a +1 +SELECT * FROM db.t2; +a +x +** Cleanup +DROP DATABASE db; === added file 'mysql-test/suite/backup_engines/t/backup_tmp_tables.test' --- a/mysql-test/suite/backup_engines/t/backup_tmp_tables.test 1970-01-01 00:00:00 +0000 +++ b/mysql-test/suite/backup_engines/t/backup_tmp_tables.test 2008-10-29 10:56:16 +0000 @@ -0,0 +1,91 @@ +# +# Test that BACKUP works correctly in the presence of temporary tables +# with the same names as tables being backed-up (BUG#33574, BUG#34903). +# + +--source include/have_innodb.inc +--source suite/backup_engines/include/backup_engine.inc + +let $bdir= `SELECT @@backupdir`; + +--echo ** Pre-cleanup +--disable_warnings +DROP DATABASE IF EXISTS db; +--error 0,1 +--remove_file $bdir/db.bkp +--enable_warnings + +--echo ** Create a database +CREATE DATABASE db; +USE db; + +--echo ** Create regular tables +CREATE TABLE t1 (a int); +CREATE TABLE t2 (a char(1)); + +--echo ** Create a view +CREATE VIEW v1 AS SELECT * FROM t1; + +--echo ** Store table's definition for later check +let $stmt1= query_get_value(SHOW CREATE TABLE t1, Create Table, 1); +let $stmt2= query_get_value(SHOW CREATE TABLE t2, Create Table, 1); + +--echo ** Insert some data into the tables +INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES ('x'); + +--echo ** Create temporary tables with the same name, but different layout +--echo ** and using different storage engines +CREATE TEMPORARY TABLE t1 (b text, c int) ENGINE=MyISAM; +CREATE TEMPORARY TABLE t2 (b int, c blob) ENGINE=InnoDB; + +--echo ** Insert data into the temporary tables +INSERT INTO t1 VALUES ('foo', 2); +INSERT INTO t2 VALUES (3, 'bar'); + +--echo ** Backup database +--replace_column 1 # +BACKUP DATABASE db TO 'db.bkp'; + +--echo ** Drop and restore the database +DROP TABLE t1; +DROP TABLE t2; +--replace_column 1 # +RESTORE FROM 'db.bkp'; + +# +# Note: Above DROP TABLE statements should be removed once BUG#30099 is fixed. +# + +--echo ** Check definitions of the tables after restore +let $stmt1a= query_get_value(SHOW CREATE TABLE t1, Create Table, 1); +let $stmt2a= query_get_value(SHOW CREATE TABLE t2, Create Table, 1); +--disable_query_log +--eval SELECT strcmp("$stmt1","$stmt1a") AS table_t1 +--eval SELECT strcmp("$stmt2","$stmt2a") AS table_t2 +--enable_query_log + +# +# Note: The above tests using strcmp() might be too sensitive, although they +# work now. Theoreticaly the details of the CREATE TABLE statement produced by +# SHOW CREATE TABLE could be different even if RESTORE works ok. If you see +# result missmatch here, analyse carefully what are the differences before +# reporting a bug. The main purpose of above checks is to ensure that RESTORE +# haven't wrongly captured definitions of the temporary tables instead of the +# regular ones. +# + +--echo ** Checking data after restore +SELECT * FROM t1; +SELECT * FROM t2; +SELECT * FROM v1; + +--echo ** Checking if restored table is seen from other connection +connect (A, localhost, root,,); +--connection A +SELECT * FROM db.t1; +SELECT * FROM db.t2; + +--echo ** Cleanup +DROP DATABASE db; +--remove_file $bdir/db.bkp === modified file 'sql/backup/be_snapshot.cc' --- a/sql/backup/be_snapshot.cc 2008-08-08 19:58:37 +0000 +++ b/sql/backup/be_snapshot.cc 2008-10-29 10:56:16 +0000 @@ -126,7 +126,15 @@ result_t Backup::get_data(Buffer &buf) // open_and_lock_tables. Otherwise, open_and_lock_tables will try to open // previously opened views and crash. locking_thd->m_thd->lex->cleanup_after_one_table_open(); - open_and_lock_tables(locking_thd->m_thd, locking_thd->tables_in_backup); + /* + The MYSQL_OPEN_SKIP_TEMPORARY flag is needed so that temporary tables are + not opened which would occulde the regular tables selected for backup + (BUG#33574). + */ + open_and_lock_tables_derived(locking_thd->m_thd, + locking_thd->tables_in_backup, + FALSE, /* do not process derived tables */ + MYSQL_OPEN_SKIP_TEMPORARY); tables_open= TRUE; } if (locking_thd->lock_state == LOCK_ACQUIRED) === modified file 'sql/backup/be_thread.cc' --- a/sql/backup/be_thread.cc 2008-07-09 07:12:43 +0000 +++ b/sql/backup/be_thread.cc 2008-10-29 10:56:16 +0000 @@ -161,7 +161,16 @@ pthread_handler_t backup_thread_for_lock killing the thread. In this case, we need to close the tables and exit. */ - if (open_and_lock_tables(thd, locking_thd->tables_in_backup)) + + /* + The MYSQL_OPEN_SKIP_TEMPORARY flag is needed so that temporary tables are + not opened which would occulde the regular tables selected for backup + (BUG#33574). + */ + if (open_and_lock_tables_derived(thd, locking_thd->tables_in_backup, + FALSE, /* do not process derived tables */ + MYSQL_OPEN_SKIP_TEMPORARY) + ) { DBUG_PRINT("info",("Online backup locking thread dying")); THD_SET_PROC_INFO(thd, "lock error"); === modified file 'sql/backup/kernel.cc' --- a/sql/backup/kernel.cc 2008-10-27 13:06:21 +0000 +++ b/sql/backup/kernel.cc 2008-10-29 10:56:16 +0000 @@ -838,11 +838,18 @@ int Backup_restore_ctx::lock_tables_for_ /* Open and lock the tables. - Note: simple_open_n_lock_tables() must be used here since we don't want - to do derived tables processing. Processing derived tables even leads - to crashes as those reported in BUG#34758. + Note 1: It is important to not do derived tables processing here. Processing + derived tables even leads to crashes as those reported in BUG#34758. + + Note 2: Skiping tmp tables is also important because otherwise a tmp table + can occlude a regular table with the same name (BUG#33574). */ - if (simple_open_n_lock_tables(m_thd,tables)) + if (open_and_lock_tables_derived(m_thd, tables, + FALSE, /* do not process derived tables */ + MYSQL_OPEN_SKIP_TEMPORARY + /* do not open tmp tables */ + ) + ) { fatal_error(ER_BACKUP_OPEN_TABLES,"RESTORE"); return m_error; === modified file 'sql/si_objects.cc' --- a/sql/si_objects.cc 2008-10-27 13:06:21 +0000 +++ b/sql/si_objects.cc 2008-10-29 10:56:16 +0000 @@ -2126,8 +2126,15 @@ bool TableObj::do_serialize(THD *thd, St /* Open the view and its base tables or views - */ - if (open_normal_and_derived_tables(thd, table_list, 0)) { + + The MYSQL_OPEN_SKIP_TEMPORARY flag is needed because TableObj always + refers to a regular table, even if a temporary table with the same name + exists in the database (see BUG#33574). + */ + if (open_normal_and_derived_tables(thd, table_list, + MYSQL_OPEN_SKIP_TEMPORARY) + ) + { close_thread_tables(thd); thd->lex->select_lex.table_list.empty(); DBUG_RETURN(TRUE);