bzr commit into mysql-6.0-backup branch (jorgen.loland:2673) Bug#34902

[Jorgen Loland]

#At file:///localhome/jl208045/mysql/mysql-6.0-backup-34902/

 2673 Jorgen Loland	2008-07-31
      Bug#34902: "Backup: crash if view depends on dropped object"
      
      Pre-fix behavior: When adding a view to the backup image fails, 
      the object representing the view is deleted but not removed 
      from the database catalog. This view object is later tried deleted
      as part of catalog delete.
                  
      Fix: Add the object and all its dependencies to dependency list
      before adding the object to the catalog. If any dependant objects
      fail to be added to dep list, the object is not added to the
      catalog.
modified:
  mysql-test/lib/mtr_report.pl
  mysql-test/r/backup_views.result
  mysql-test/t/backup_views.test
  sql/backup/backup_info.cc

per-file comments:
  mysql-test/lib/mtr_report.pl
    backup_views.test deliberately triggers errors "Backup: Failed to add view..." and
"Restore: Could not restore view..." Ignore these errors in the test.
  mysql-test/r/backup_views.result
    Added results from new test case: backup database with missing view dependencies.
  mysql-test/t/backup_views.test
    Added test case: backup database with missing view dependencies.
  sql/backup/backup_info.cc
    In Backup_info::add_db_object
    The object is added to catalog after the object and all dependencies have been added
to the dependency list. If any of the dependent objects cannot be added to the dep list,
the object is not added to the catalog.
=== modified file 'mysql-test/lib/mtr_report.pl'
--- a/mysql-test/lib/mtr_report.pl	2008-07-09 07:12:43 +0000
+++ b/mysql-test/lib/mtr_report.pl	2008-07-31 11:10:54 +0000
@@ -340,6 +340,13 @@ sub mtr_report_stats ($) {
 		  /Restore: Tablespace .* needed by tables being restored has changed on the server/
 		) or
 		
+		# The views test triggers errors below on purpose
+		($testname eq 'main.backup_views') and
+		(
+		  /Backup: Failed to add view/ or
+		  /Restore: Could not restore view/
+		) or
+ 	 
 		# ignore warning generated when backup engine selection algorithm is tested
 		($testname eq 'main.backup_no_be') and /Backup: Cannot create backup engine/ or
 		# ignore warnings generated when backup privilege is tested

=== modified file 'mysql-test/r/backup_views.result'
--- a/mysql-test/r/backup_views.result	2008-06-12 09:55:35 +0000
+++ b/mysql-test/r/backup_views.result	2008-07-31 11:10:54 +0000
@@ -79,6 +79,7 @@ USE bup_db1;
 *******Creating View from database bup_db2**********
 CREATE VIEW v5 AS SELECT * FROM bup_db2.t2;
 ******Creating View v6********
+CREATE VIEW v6 AS SELECT education,gender FROM bup_db2.v2, t5  WHERE cand_age=age;
 USE bup_db1;
 SELECT * FROM t1;
 id	name	city
@@ -134,6 +135,16 @@ idno	age	education
 6	30	Doctor
 7	31	Lawyer
 8	27	undergrad
+SELECT * FROM v6;
+education	gender
+BS	F
+BE	F
+school	M
+MS	F
+PHD	M
+Doctor	F
+Lawyer	M
+undergrad	M
 excercise objects of bup_db2
 USE bup_db2;
 SELECT * FROM t2;
@@ -185,6 +196,8 @@ Tables_in_bup_db1	v1
 Table_type	VIEW
 Tables_in_bup_db1	v5
 Table_type	VIEW
+Tables_in_bup_db1	v6
+Table_type	VIEW
 Tables_in_bup_db1	vcomb
 Table_type	VIEW
 SHOW FULL TABLES FROM bup_db2;;
@@ -224,6 +237,13 @@ backup_id
 dropping  database.
 DROP DATABASE bup_db1;
 DROP DATABASE bup_db2;
+restore database with view dependency to other, non-existing db
+RESTORE FROM 'bup_objectview1.bak';
+ERROR 42S02: Table 'bup_db2.t2' doesn't exist
+DROP DATABASE bup_db1;
+RESTORE FROM 'bup_objectview2.bak';
+ERROR 42S02: Table 'bup_db1.t3' doesn't exist
+DROP DATABASE bup_db2;
 RESTORE FROM 'bup_objectview.bak';
 backup_id
 #
@@ -242,6 +262,8 @@ Tables_in_bup_db1	v1
 Table_type	VIEW
 Tables_in_bup_db1	v5
 Table_type	VIEW
+Tables_in_bup_db1	v6
+Table_type	VIEW
 Tables_in_bup_db1	vcomb
 Table_type	VIEW
 SHOW FULL TABLES FROM bup_db2;;
@@ -324,6 +346,16 @@ idno	age	education
 6	30	Doctor
 7	31	Lawyer
 8	27	undergrad
+SELECT * FROM v6;
+education	gender
+BS	F
+BE	F
+school	M
+MS	F
+PHD	M
+Doctor	F
+Lawyer	M
+undergrad	M
 excercise objects of bup_db2
 use bup_db2;
 SELECT * FROM t2;
@@ -447,6 +479,55 @@ id	name	city
 6	aa6	RR6
 8	aa8	RR8
 
+*** ENTER Backup of database with missing view dependency should fail but not crash
server
+
+initializing test
+DROP DATABASE bup_db1;
+DROP DATABASE bup_db2;
+RESTORE FROM 'bup_objectview.bak';
+backup_id
+#
+USE bup_db1;
+SELECT * FROM t1;
+id	name	city
+1	aa1	RR1
+2	aa2	RR2
+3	aa3	RR3
+4	aa4	RR4
+5	aa5	RR5
+6	aa6	RR6
+7	aa7	RR7
+8	aa8	RR8
+SELECT * FROM v1;
+id	name	city
+1	aa1	RR1
+2	aa2	RR2
+3	aa3	RR3
+4	aa4	RR4
+5	aa5	RR5
+6	aa6	RR6
+7	aa7	RR7
+8	aa8	RR8
+DROP TABLE t1;
+
+Testing backup with missing view dependency in same db
+
+SELECT * FROM v1;
+ERROR HY000: View 'bup_db1.v1' references invalid table(s) or column(s) or function(s) or
definer/invoker of view lack rights to use them
+BACKUP DATABASE bup_db1 TO 'bup_shouldfail1.bak';
+ERROR HY000: Failed to add view `bup_db1`.`v1` to the catalog
+
+Testing backup with missing view dependency in other db
+
+USE bup_db2;
+SELECT * from v3;
+ERROR HY000: View 'bup_db2.v3' references invalid table(s) or column(s) or function(s) or
definer/invoker of view lack rights to use them
+BACKUP DATABASE bup_db2 TO 'bup_shouldfail2.bak';
+ERROR HY000: Failed to add view `bup_db2`.`v3` to the catalog
+
+*** EXIT Backup of database with missing view dependency
+
+
 ***  DROP bup_db1, bup_db2 DATABASE ****
 
 DROP DATABASE bup_db1;

=== modified file 'mysql-test/t/backup_views.test'
--- a/mysql-test/t/backup_views.test	2008-06-12 09:55:35 +0000
+++ b/mysql-test/t/backup_views.test	2008-07-31 11:10:54 +0000
@@ -124,9 +124,7 @@ CREATE VIEW v5 AS SELECT * FROM bup_db2.
 
 --echo ******Creating View v6********
 
-#Bug#36213 Restore fails for a database that has views created using another database .
-
-#CREATE VIEW v6 AS SELECT education,gender FROM bup_db2.v2, t5  WHERE cand_age=age;
+CREATE VIEW v6 AS SELECT education,gender FROM bup_db2.v2, t5  WHERE cand_age=age;
 
 #Excercise the objects of bup_db1
 
@@ -144,7 +142,7 @@ SELECT * FROM vcomb;
 
 SELECT * FROM v5;
 
-#SELECT * FROM v6;
+SELECT * FROM v6;
 
 --echo excercise objects of bup_db2
 
@@ -188,11 +186,21 @@ DROP DATABASE bup_db2;
 #RESTORE FROM bup_objectview.bak;
 
 #Individual databases cannot be restored because of VIEW DEPENDENCY
+--echo restore database with view dependency to other, non-existing db
+
+--error ER_NO_SUCH_TABLE
+RESTORE FROM 'bup_objectview1.bak'; 
 
-#--error 1146
-#RESTORE FROM 'bup_objectview1.bak';
-#--error 1146
-#RESTORE FROM 'bup_objectview2.bak';
+# An incomplete bup_db1 was created by the failing restore operation.
+# Remove it before trying restore of bup_db2.
+DROP DATABASE bup_db1;
+
+--error ER_NO_SUCH_TABLE
+RESTORE FROM 'bup_objectview2.bak';
+
+# An incomplete bup_db2 was created by the failing restore operation.
+# Remove it before reverting to the working backup image
+DROP DATABASE bup_db2;
 
 replace_column 1 #;
 RESTORE FROM 'bup_objectview.bak';
@@ -222,7 +230,7 @@ SELECT * FROM vcomb;
 
 SELECT * FROM v5;
 
-#SELECT * FROM v6;
+SELECT * FROM v6;
 --echo excercise objects of bup_db2
 use bup_db2;
 SELECT * FROM t2;
@@ -281,6 +289,58 @@ SELECT * FROM v3;
 USE bup_db1;
 SELECT * FROM t1;
 
+
+###############
+--echo 
+--echo *** ENTER Backup of database with missing view dependency should fail but not
crash server
+--echo 
+
+--echo initializing test
+
+# start with the backed up database
+DROP DATABASE bup_db1;
+DROP DATABASE bup_db2;
+
+replace_column 1 #;
+RESTORE FROM 'bup_objectview.bak';
+
+# check that table t1 and v1 are initially correct
+USE bup_db1;
+SELECT * FROM t1;
+SELECT * FROM v1;
+
+DROP TABLE t1;
+
+--echo
+--echo Testing backup with missing view dependency in same db
+--echo 
+
+# v1 selects from t1, and select now reports error
+--error ER_VIEW_INVALID
+SELECT * FROM v1;
+
+# try to backup - v1 selects from t1 and backup should now fail
+--error ER_BACKUP_CATALOG_ADD_VIEW
+BACKUP DATABASE bup_db1 TO 'bup_shouldfail1.bak';
+
+--echo
+--echo Testing backup with missing view dependency in other db
+--echo 
+
+USE bup_db2;
+--error ER_VIEW_INVALID
+SELECT * from v3;
+
+# try to backup - v3 selects from bup_db1.t1 and backup should now fail
+--error ER_BACKUP_CATALOG_ADD_VIEW
+BACKUP DATABASE bup_db2 TO 'bup_shouldfail2.bak';
+
+--echo 
+--echo *** EXIT Backup of database with missing view dependency
+--echo 
+
+###############
+
 # Test cleanup section
 
 --echo
@@ -298,4 +358,10 @@ DROP DATABASE bup_db2;
 
 --remove_file $MYSQLTEST_VARDIR/master-data/bup_objectview2.bak
 --remove_file $MYSQLTEST_VARDIR/master-data/bup_objectview3.bak
+
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_shouldfail1.bak
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_shouldfail2.bak
+
 #BUG#35249 Mysql server crash for delete operation followed by backup for Default
Drivers.

=== modified file 'sql/backup/backup_info.cc'
--- a/sql/backup/backup_info.cc	2008-07-09 07:12:43 +0000
+++ b/sql/backup/backup_info.cc	2008-07-31 11:10:54 +0000
@@ -964,9 +964,9 @@ error:
   @param[in] type type of the object
   @param[in] obj  the object
 
-  The object is also added to the dependency list with @c add_to_dep_list() 
-  method. If it is a view, its dependencies are handled first using 
-  @c add_view_deps().
+  The object is added both to the dependency list with @c
+  add_to_dep_list() method and to the catalogue. If it is a view, its
+  dependencies are handled first using @c add_view_deps().
 
   @returns Pointer to @c Image_info::Dbobj instance storing information 
   about the object or NULL in case of error.  
@@ -998,16 +998,6 @@ Backup_info::add_db_object(Db &db, const
 
   }
 
-  Dbobj *o= Image_info::add_db_object(db, type, *name, pos);
-  
-  if (!o)
-  {
-    m_ctx.fatal_error(error, db.name().ptr(), name->ptr());
-    return NULL;
-  }
-
-  o->m_obj_ptr= obj;
-
   /* 
     Add new object to the dependency list. If it is a view, add its
     dependencies first.
@@ -1026,15 +1016,6 @@ Backup_info::add_db_object(Db &db, const
     return NULL;
   }
 
-  /* 
-    Store a pointer to the catalogue item in the dep. list node. If this node
-    was a placeholder inserted into the list before, now it will be filled with
-    the object we are adding to the catalogue.
-   */
-
-  DBUG_ASSERT(n);
-  n->obj= o;  
-
   /*
     If a new node was created, it must be added to the dependency list with
     add_to_dep_list(). However, if the object is a view, we must first add 
@@ -1053,6 +1034,36 @@ Backup_info::add_db_object(Db &db, const
     add_to_dep_list(type, n);
   } 
 
+  /*
+    The object has now been added to the dependancy list. If it is a
+    view, all dependant objects have also been successfully added to
+    the dependency list. The object can now be added to the cataloge
+    and then be linked to from the node in the dep list. Adding to dep
+    list before adding to catalogue ensures that an object will not be
+    added to catalogue if there are problems with it's dependant
+    objects.
+   */
+
+  // Add object to catalogue
+  Dbobj *o= Image_info::add_db_object(db, type, *name, pos);
+  
+  if (!o)
+  {
+    m_ctx.fatal_error(error, db.name().ptr(), name->ptr());
+    return NULL;
+  }
+
+  o->m_obj_ptr= obj;
+
+  /* 
+    Store a pointer to the catalogue item in the dep. list node. If this node
+    was a placeholder inserted into the list before, now it will be filled with
+    the object we are adding to the catalogue.
+   */
+
+  DBUG_ASSERT(n);
+  n->obj= o;  
+
   DBUG_PRINT("backup",("Added object %s of type %d from database %s (pos=%lu)",
                        name->ptr(), type, db.name().ptr(), pos));
   return o;