bzr commit into mysql-5.1-bugteam branch (ramil:2737) Bug#33094

[Ramil Kalimullin]

#At file:///home/ram/mysql/b33094-5.1-bugteam/ based on revid:patrick.crews@stripped35-8crjgzh4xp8qz2y1

 2737 Ramil Kalimullin	2009-01-14
      Fix for
      bug#33094: Error in upgrading from 5.0 to 5.1 when table contains
      triggers
      and
      #41385: Crash when attempting to repair a #mysql50# upgraded table
      #with
      triggers.
      
      Problem:
      1. trigger code didn't assume a table name may have
      a "#mysql50#" prefix, that may lead to a failing ASSERT().
      2. "ALTER DATABASE ... UPGRADE DATA DIRECTORY NAME" failed
      for databases with "#mysql50#" prefix if any trigger.
      3. mysqlcheck --fix-table-name didn't use UTF8 as a default
      character set that resulted in (parsing) errors for tables with
      non-latin symbols in their names and definitions of triggers.
      
      Fix:
      1. properly handle table/database names with "#mysql50#" prefix.
      2. handle --default-character-set mysqlcheck option;
      if mysqlcheck is launched with --fix-table-name or --fix-db-name
      set default character set to UTF8 if no --default-character-set
      option given.
      
      Note: if given --fix-table-name or --fix-db-name option,
      without --default-character-set mysqlcheck option
      default character set is UTF8.
modified:
  client/mysqlcheck.c
  mysql-test/r/mysqlcheck.result
  mysql-test/t/mysqlcheck.test
  sql/sql_trigger.cc
  sql/sql_trigger.h

per-file messages:
  client/mysqlcheck.c
    Fix for
    bug#33094: Error in upgrading from 5.0 to 5.1 when table contains
    triggers
    and
    #41385: Crash when attempting to repair a #mysql50# upgraded table with
    triggers.
      - check and set default charset if --default-character-set option
    given.
      - set default charset to "utf8" if there's
    --fix-table-name or --fix-db-name and no --default-character-set.
  mysql-test/r/mysqlcheck.result
    Fix for
    bug#33094: Error in upgrading from 5.0 to 5.1 when table contains
    triggers
    and
    #41385: Crash when attempting to repair a #mysql50# upgraded table
    #with
    triggers.
      - test result.
  mysql-test/t/mysqlcheck.test
    Fix for
    bug#33094: Error in upgrading from 5.0 to 5.1 when table contains
    triggers
    and
    #41385: Crash when attempting to repair a #mysql50# upgraded table
    #with
    triggers.
      - test case.
  sql/sql_trigger.cc
    Fix for
    bug#33094: Error in upgrading from 5.0 to 5.1 when table contains
    triggers
    and
    #41385: Crash when attempting to repair a #mysql50# upgraded table
    #with
    triggers.
      - Table_triggers_list::check_n_load() - checking triggers assume 
        a table name given may have "#mysql50#" prefix in some cases.
      - Table_triggers_list::change_table_name_in_triggers() - 
        create .TRG file in new database directory and delete it in old one,
        as they may differ in case of 
        "ALTER DATABASE ... UPGRADE DATA DIRECTORY NAME"
      - Table_triggers_list::remove_table_trigger_files() - introduced
        to delete all table triggers' .TRN files.
      - Table_triggers_list::change_table_name() - allow changing trigger's
        database in case of its upgrading,
        remove stale .TRN files in #mysql50#dbname directory after such upgrade.
  sql/sql_trigger.h
    Fix for
    bug#33094: Error in upgrading from 5.0 to 5.1 when table contains
    triggers
    and
    #41385: Crash when attempting to repair a #mysql50# upgraded table
    #with
    triggers.
      - Table_triggers_list::remove_table_trigger_files() - introduced.
      - Table_triggers_list::change_table_name_in_triggers() - added
        old database name parameter.
=== modified file 'client/mysqlcheck.c'
--- a/client/mysqlcheck.c	2008-11-14 09:48:01 +0000
+++ b/client/mysqlcheck.c	2009-01-13 20:10:16 +0000
@@ -40,15 +40,13 @@ static uint verbose = 0, opt_mysql_port=
 static int my_end_arg;
 static char * opt_mysql_unix_port = 0;
 static char *opt_password = 0, *current_user = 0, 
-	    *default_charset = (char *)MYSQL_DEFAULT_CHARSET_NAME,
-	    *current_host = 0;
+	    *default_charset= 0, *current_host= 0;
 static int first_error = 0;
 DYNAMIC_ARRAY tables4repair;
 #ifdef HAVE_SMEM
 static char *shared_memory_base_name=0;
 #endif
 static uint opt_protocol=0;
-static CHARSET_INFO *charset_info= &my_charset_latin1;
 
 enum operations { DO_CHECK, DO_REPAIR, DO_ANALYZE, DO_OPTIMIZE, DO_UPGRADE };
 
@@ -282,12 +280,10 @@ get_one_option(int optid, const struct m
     break;
   case OPT_FIX_DB_NAMES:
     what_to_do= DO_UPGRADE;
-    default_charset= (char*) "utf8";
     opt_databases= 1;
     break;
   case OPT_FIX_TABLE_NAMES:
     what_to_do= DO_UPGRADE;
-    default_charset= (char*) "utf8";
     break;
   case 'p':
     if (argument)
@@ -367,11 +363,20 @@ static int get_options(int *argc, char *
       what_to_do = DO_CHECK;
   }
 
-  /* TODO: This variable is not yet used */
-  if (strcmp(default_charset, charset_info->csname) &&
-      !(charset_info= get_charset_by_csname(default_charset, 
-  					    MY_CS_PRIMARY, MYF(MY_WME))))
-      exit(1);
+  /*
+    If there's no --default-character-set option given with
+    --fix-table-name or --fix-db-name set the default character set to "utf8".
+  */
+  if (!default_charset && (opt_fix_db_names || opt_fix_table_names))
+  {
+    default_charset= (char*) "utf8";
+  }
+  if (default_charset && !get_charset_by_csname(default_charset, MY_CS_PRIMARY,
+                                                MYF(MY_WME)))
+  {
+    printf("Unsupported character set: %s\n", default_charset);
+    return 1;
+  }
   if (*argc > 0 && opt_alldbs)
   {
     printf("You should give only options, no arguments at all, with option\n");
@@ -779,6 +784,8 @@ static int dbConnect(char *host, char *u
   if (shared_memory_base_name)
     mysql_options(&mysql_connection,MYSQL_SHARED_MEMORY_BASE_NAME,shared_memory_base_name);
 #endif
+  if (default_charset)
+    mysql_options(&mysql_connection, MYSQL_SET_CHARSET_NAME, default_charset);
   if (!(sock = mysql_real_connect(&mysql_connection, host, user, passwd,
          NULL, opt_mysql_port, opt_mysql_unix_port, 0)))
   {

=== modified file 'mysql-test/r/mysqlcheck.result'
--- a/mysql-test/r/mysqlcheck.result	2008-11-14 09:48:01 +0000
+++ b/mysql-test/r/mysqlcheck.result	2009-01-13 20:10:16 +0000
@@ -130,3 +130,46 @@ v1
 v-1
 drop view v1, `v-1`;
 drop table t1;
+SET NAMES utf8;
+CREATE TABLE `#mysql50#@` (a INT);
+SHOW TABLES;
+Tables_in_test
+#mysql50#@
+SET NAMES DEFAULT;
+mysqlcheck --fix-table-names --databases test
+SET NAMES utf8;
+SHOW TABLES;
+Tables_in_test
+@
+DROP TABLE `@`;
+CREATE TABLE `я` (a INT);
+SET NAMES DEFAULT;
+mysqlcheck --default-character-set="latin1" --databases test
+test.?
+Error    : Table 'test.?' doesn't exist
+error    : Corrupt
+mysqlcheck --default-character-set="utf8" --databases test
+test.я                                            OK
+SET NAMES utf8;
+DROP TABLE `я`;
+SET NAMES DEFAULT;
+CREATE DATABASE `#mysql50#a@b`;
+USE `#mysql50#a@b`;
+CREATE TABLE `#mysql50#c@d` (a INT);
+SHOW TRIGGERS;
+Trigger	Event	Table	Statement	Timing	Created	sql_mode	Definer	character_set_client	collation_connection	Database Collation
+tr1	INSERT	#mysql50#c@d	SET NEW.a = 10 * NEW.a	BEFORE	NULL		root@localhost	latin1	latin1_swedish_ci	latin1_swedish_ci
+Warnings:
+Warning	1603	Triggers for table `#mysql50#a@b`.`#mysql50#c@d` have no creation context
+mysqlcheck --fix-db-names --fix-table-names --all-databases
+USE `a@b`;
+SHOW TRIGGERS;
+Trigger	Event	Table	Statement	Timing	Created	sql_mode	Definer	character_set_client	collation_connection	Database Collation
+tr1	INSERT	c@d	SET NEW.a = 10 * NEW.a	BEFORE	NULL		root@localhost	utf8	utf8_general_ci	latin1_swedish_ci
+INSERT INTO `c@d` VALUES (2), (1);
+SELECT * FROM `c@d`;
+a
+20
+10
+USE test;
+End of 5.1 tests

=== modified file 'mysql-test/t/mysqlcheck.test'
--- a/mysql-test/t/mysqlcheck.test	2008-11-14 09:48:01 +0000
+++ b/mysql-test/t/mysqlcheck.test	2009-01-13 20:10:16 +0000
@@ -112,3 +112,58 @@ show tables;
 show tables;
 drop view v1, `v-1`;
 drop table t1;
+
+#
+# Bug #33094: Error in upgrading from 5.0 to 5.1 when table contains triggers
+# Bug #41385: Crash when attempting to repair a #mysql50# upgraded table with
+#             triggers
+#
+SET NAMES utf8;
+CREATE TABLE `#mysql50#@` (a INT);
+SHOW TABLES;
+SET NAMES DEFAULT;
+--echo mysqlcheck --fix-table-names --databases test
+--exec $MYSQL_CHECK --fix-table-names --databases test
+SET NAMES utf8;
+SHOW TABLES;
+DROP TABLE `@`;
+
+CREATE TABLE `я` (a INT);
+SET NAMES DEFAULT;
+--echo mysqlcheck --default-character-set="latin1" --databases test
+--exec $MYSQL_CHECK --default-character-set="latin1" --databases test
+--echo mysqlcheck --default-character-set="utf8" --databases test
+--exec $MYSQL_CHECK --default-character-set="utf8" --databases test
+SET NAMES utf8;
+DROP TABLE `я`;
+SET NAMES DEFAULT;
+
+CREATE DATABASE `#mysql50#a@b`;
+USE `#mysql50#a@b`;
+CREATE TABLE `#mysql50#c@d` (a INT);
+# Create 5.0 like trigger
+--write_file $MYSQLTEST_VARDIR/master-data/a@b/c@strippedG
+TYPE=TRIGGERS
+triggers='CREATE DEFINER=`root`@`localhost` TRIGGER tr1 BEFORE INSERT ON `c@d` FOR EACH ROW SET NEW.a = 10 * NEW.a'
+sql_modes=0
+definers='root@localhost'
+EOF
+--write_file $MYSQLTEST_VARDIR/master-data/a@b/tr1.TRN
+TYPE=TRIGGERNAME
+trigger_table=c@d
+EOF
+SHOW TRIGGERS;
+
+--echo mysqlcheck --fix-db-names --fix-table-names --all-databases
+--exec $MYSQL_CHECK --fix-db-names --fix-table-names --all-databases
+
+USE `a@b`;
+SHOW TRIGGERS;
+
+INSERT INTO `c@d` VALUES (2), (1);
+SELECT * FROM `c@d`;
+
+#DROP DATABASE `a@b`;
+USE test;
+
+--echo End of 5.1 tests

=== modified file 'sql/sql_trigger.cc'
--- a/sql/sql_trigger.cc	2008-11-14 17:37:27 +0000
+++ b/sql/sql_trigger.cc	2009-01-13 20:10:16 +0000
@@ -1368,15 +1368,24 @@ bool Table_triggers_list::check_n_load(T
 
         if (triggers->on_table_names_list.push_back(on_table_name, &table->mem_root))
           goto err_with_lex_cleanup;
-
+#ifndef DBUG_OFF
         /*
           Let us check that we correctly update trigger definitions when we
           rename tables with triggers.
+          
+          In special cases like "RENAME TABLE `#mysql50#somename` TO `somename`"
+          we might be given table name with "#mysql50#" prefix (and
+          trigger's definiton contains un-prefixed version of the same name).
+          To remove this prefix we use tablename_to_filename().
         */
-        DBUG_ASSERT(!my_strcasecmp(table_alias_charset, lex.query_tables->db, db) &&
-                    !my_strcasecmp(table_alias_charset, lex.query_tables->table_name,
-                                   table_name));
 
+        char fname[NAME_LEN + 1];
+        DBUG_ASSERT(!my_strcasecmp(table_alias_charset, lex.query_tables->db, db) &&
+                    (!my_strcasecmp(table_alias_charset, lex.query_tables->table_name,
+                                    table_name) ||
+                     (tablename_to_filename(table_name, fname, sizeof(fname)) &&
+                      !my_strcasecmp(table_alias_charset, lex.query_tables->table_name, fname))));
+#endif
         if (names_only)
         {
           lex_end(&lex);
@@ -1625,6 +1634,40 @@ bool add_table_for_trigger(THD *thd,
 
 
 /**
+  Deletes all triggers' .TRN files for a table.
+
+  @param path     FN_REFLEN byte buffer used for
+                  constructing path to .TRN files
+  @param db       table's database name
+
+  @retval
+    FALSE   success
+  @retval
+    TRUE    error
+*/
+
+bool Table_triggers_list::remove_table_trigger_files(char *path, const char *db)
+{
+  bool result= FALSE;
+  LEX_STRING *trigger;
+  List_iterator_fast<LEX_STRING> it_name(names_list);
+
+  while ((trigger= it_name++))
+  {
+    if (rm_trigname_file(path, db, trigger->str))
+    {
+      /*
+        Instead of immediately bailing out with error if we were unable
+        to remove .TRN file we will try to drop other files.
+      */
+      result= TRUE;
+    }
+  }
+  return result;
+}
+
+
+/**
   Drop all triggers for table.
 
   @param thd      current thread context
@@ -1643,7 +1686,6 @@ bool add_table_for_trigger(THD *thd,
 bool Table_triggers_list::drop_all_triggers(THD *thd, char *db, char *name)
 {
   TABLE table;
-  char path[FN_REFLEN];
   bool result= 0;
   DBUG_ENTER("drop_all_triggers");
 
@@ -1657,28 +1699,14 @@ bool Table_triggers_list::drop_all_trigg
   }
   if (table.triggers)
   {
-    LEX_STRING *trigger;
-    List_iterator_fast<LEX_STRING> it_name(table.triggers->names_list);
-
-    while ((trigger= it_name++))
-    {
-      if (rm_trigname_file(path, db, trigger->str))
-      {
-        /*
-          Instead of immediately bailing out with error if we were unable
-          to remove .TRN file we will try to drop other files.
-        */
-        result= 1;
-        continue;
-      }
-    }
-
+    char path[FN_REFLEN];
+    /* Remove .TRN files */
+    result= table.triggers->remove_table_trigger_files(path, db);
+    /* Remove .TRG file */
     if (rm_trigger_file(path, db, name))
-    {
-      result= 1;
-      goto end;
-    }
+      result= TRUE;
   }
+
 end:
   if (table.triggers)
     delete table.triggers;
@@ -1704,7 +1732,8 @@ end:
 
 bool
 Table_triggers_list::change_table_name_in_triggers(THD *thd,
-                                                   const char *db_name,
+                                                   const char *old_db_name,
+                                                   const char *new_db_name,
                                                    LEX_STRING *old_table_name,
                                                    LEX_STRING *new_table_name)
 {
@@ -1757,11 +1786,11 @@ Table_triggers_list::change_table_name_i
   if (thd->is_fatal_error)
     return TRUE; /* OOM */
 
-  if (save_trigger_file(this, db_name, new_table_name->str))
+  if (save_trigger_file(this, new_db_name, new_table_name->str))
     return TRUE;
-  if (rm_trigger_file(path_buff, db_name, old_table_name->str))
+  if (rm_trigger_file(path_buff, old_db_name, old_table_name->str))
   {
-    (void) rm_trigger_file(path_buff, db_name, new_table_name->str);
+    (void) rm_trigger_file(path_buff, new_db_name, new_table_name->str);
     return TRUE;
   }
   return FALSE;
@@ -1840,6 +1869,7 @@ bool Table_triggers_list::change_table_n
 {
   TABLE table;
   bool result= 0;
+  bool upgrading50to51= FALSE; 
   LEX_STRING *err_trigname;
   DBUG_ENTER("change_table_name");
 
@@ -1877,14 +1907,24 @@ bool Table_triggers_list::change_table_n
       moving table with them between two schemas raises too many questions.
       (E.g. what should happen if in new schema we already have trigger
        with same name ?).
+       
+      In case of "ALTER DATABASE `#mysql50#db1` UPGRADE DATA DIRECTORY NAME"
+      we will be given table name with "#mysql50#" prefix
+      To remove this prefix we use tablename_to_filename().
     */
     if (my_strcasecmp(table_alias_charset, db, new_db))
     {
-      my_error(ER_TRG_IN_WRONG_SCHEMA, MYF(0));
-      result= 1;
-      goto end;
+      char dbname[NAME_LEN + 1];
+      tablename_to_filename(db, dbname, sizeof(dbname));
+      if (my_strcasecmp(table_alias_charset, dbname, new_db))
+      {
+        my_error(ER_TRG_IN_WRONG_SCHEMA, MYF(0));
+        result= 1;
+        goto end;
+      }
+      upgrading50to51= TRUE;
     }
-    if (table.triggers->change_table_name_in_triggers(thd, db,
+    if (table.triggers->change_table_name_in_triggers(thd, db, new_db,
                                                       &old_table_name,
                                                       &new_table_name))
     {
@@ -1892,7 +1932,7 @@ bool Table_triggers_list::change_table_n
       goto end;
     }
     if ((err_trigname= table.triggers->change_table_name_in_trignames(
-                                         db, &new_table_name, 0)))
+                                         new_db, &new_table_name, 0)))
     {
       /*
         If we were unable to update one of .TRN files properly we will
@@ -1900,16 +1940,23 @@ bool Table_triggers_list::change_table_n
         We assume that we will be able to undo our changes without errors
         (we can't do much if there will be an error anyway).
       */
-      (void) table.triggers->change_table_name_in_trignames(db,
+      (void) table.triggers->change_table_name_in_trignames(new_db,
                                                             &old_table_name,
                                                             err_trigname);
-      (void) table.triggers->change_table_name_in_triggers(thd, db,
+      (void) table.triggers->change_table_name_in_triggers(thd, db, new_db,
                                                            &new_table_name,
                                                            &old_table_name);
       result= 1;
       goto end;
     }
+    if (upgrading50to51)
+    {
+      char path[FN_REFLEN];
+      /* Remove .TRN files in old database */
+      result= table.triggers->remove_table_trigger_files(path, db);
+    }
   }
+  
 end:
   delete table.triggers;
   free_root(&table.mem_root, MYF(0));

=== modified file 'sql/sql_trigger.h'
--- a/sql/sql_trigger.h	2007-10-16 20:11:50 +0000
+++ b/sql/sql_trigger.h	2009-01-13 20:10:16 +0000
@@ -150,9 +150,11 @@ private:
                                              LEX_STRING *new_table_name,
                                              LEX_STRING *stopper);
   bool change_table_name_in_triggers(THD *thd,
-                                     const char *db_name,
+                                     const char *old_db_name,
+                                     const char *new_db_name,
                                      LEX_STRING *old_table_name,
                                      LEX_STRING *new_table_name);
+  bool remove_table_trigger_files(char *path, const char *db);
 };
 
 extern const LEX_STRING trg_action_time_type_names[];