List:Commits« Previous MessageNext Message »
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
View as plain text  
#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);

Thread
bzr commit into mysql-6.0-backup branch (Rafal.Somla:2718) Bug#33574Bug#34903Rafal Somla29 Oct
  • Re: bzr commit into mysql-6.0-backup branch (Rafal.Somla:2718)Bug#33574 Bug#34903Jørgen Løland29 Oct