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.
17 years ago · 347762946e
7 changed files with 241 additions and 40 deletions
--- a/client/mysqlcheck.c
+++ b/client/mysqlcheck.c
@ -40,15 +40,13 @@ static uint verbose = 0, opt_mysql_port=0;
 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 my_option *opt __attribute__((unused)),
    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 ***argv)
      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 *user, char *passwd)
  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)))
  {
--- a/mysql-test/r/mysqlcheck.result
+++ b/mysql-test/r/mysqlcheck.result
@ -130,3 +130,58 @@ 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);
+CREATE TABLE t1 (a INT);
+SELECT * FROM INFORMATION_SCHEMA.TRIGGERS
+WHERE TRIGGER_SCHEMA="#mysql50#a@b" ORDER BY trigger_name;
+TRIGGER_CATALOG	TRIGGER_SCHEMA	TRIGGER_NAME	EVENT_MANIPULATION	EVENT_OBJECT_CATALOG	EVENT_OBJECT_SCHEMA	EVENT_OBJECT_TABLE	ACTION_ORDER	ACTION_CONDITION	ACTION_STATEMENT	ACTION_ORIENTATION	ACTION_TIMING	ACTION_REFERENCE_OLD_TABLE	ACTION_REFERENCE_NEW_TABLE	ACTION_REFERENCE_OLD_ROW	ACTION_REFERENCE_NEW_ROW	CREATED	SQL_MODE	DEFINER	CHARACTER_SET_CLIENT	COLLATION_CONNECTION	DATABASE_COLLATION
+NULL	#mysql50#a@b	tr1	INSERT	NULL	#mysql50#a@b	#mysql50#c@d	0	NULL	SET NEW.a = 10 * NEW.a	ROW	BEFORE	NULL	NULL	OLD	NEW	NULL		root@localhost	latin1	latin1_swedish_ci	latin1_swedish_ci
+NULL	#mysql50#a@b	tr2	INSERT	NULL	#mysql50#a@b	t1	0	NULL	SET NEW.a = 100 * NEW.a	ROW	BEFORE	NULL	NULL	OLD	NEW	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
+Warning	1603	Triggers for table `#mysql50#a@b`.`t1` have no creation context
+mysqlcheck --fix-db-names --fix-table-names --all-databases
+USE `a@b`;
+SELECT * FROM INFORMATION_SCHEMA.TRIGGERS
+WHERE TRIGGER_SCHEMA="a@b" ORDER BY trigger_name;
+TRIGGER_CATALOG	TRIGGER_SCHEMA	TRIGGER_NAME	EVENT_MANIPULATION	EVENT_OBJECT_CATALOG	EVENT_OBJECT_SCHEMA	EVENT_OBJECT_TABLE	ACTION_ORDER	ACTION_CONDITION	ACTION_STATEMENT	ACTION_ORIENTATION	ACTION_TIMING	ACTION_REFERENCE_OLD_TABLE	ACTION_REFERENCE_NEW_TABLE	ACTION_REFERENCE_OLD_ROW	ACTION_REFERENCE_NEW_ROW	CREATED	SQL_MODE	DEFINER	CHARACTER_SET_CLIENT	COLLATION_CONNECTION	DATABASE_COLLATION
+NULL	a@b	tr1	INSERT	NULL	a@b	c@d	0	NULL	SET NEW.a = 10 * NEW.a	ROW	BEFORE	NULL	NULL	OLD	NEW	NULL		root@localhost	utf8	utf8_general_ci	latin1_swedish_ci
+NULL	a@b	tr2	INSERT	NULL	a@b	t1	0	NULL	SET NEW.a = 100 * NEW.a	ROW	BEFORE	NULL	NULL	OLD	NEW	NULL		root@localhost	utf8	utf8_general_ci	latin1_swedish_ci
+INSERT INTO `c@d` VALUES (2), (1);
+SELECT * FROM `c@d`;
+a
+20
+10
+INSERT INTO t1 VALUES (3), (5);
+SELECT * FROM t1;
+a
+300
+500
+DROP DATABASE `a@b`;
+USE test;
+End of 5.1 tests
--- a/mysql-test/t/mysqlcheck.test
+++ b/mysql-test/t/mysqlcheck.test
@ -112,3 +112,76 @@ 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 TABLE t1 (a INT);
+
+# Create 5.0 like triggers
+--write_file $MYSQLTEST_VARDIR/master-data/a@b/c@d.TRG
+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
+--write_file $MYSQLTEST_VARDIR/master-data/a@b/t1.TRG
+TYPE=TRIGGERS
+triggers='CREATE DEFINER=`root`@`localhost` TRIGGER tr2 BEFORE INSERT ON `a@b`.t1 FOR EACH ROW SET NEW.a = 100 * NEW.a'
+sql_modes=0
+definers='root@localhost'
+EOF
+--write_file $MYSQLTEST_VARDIR/master-data/a@b/tr2.TRN
+TYPE=TRIGGERNAME
+trigger_table=t1
+EOF
+
+SELECT * FROM INFORMATION_SCHEMA.TRIGGERS
+  WHERE TRIGGER_SCHEMA="#mysql50#a@b" ORDER BY trigger_name;
+
+--echo mysqlcheck --fix-db-names --fix-table-names --all-databases
+--exec $MYSQL_CHECK --fix-db-names --fix-table-names --all-databases
+
+USE `a@b`;
+SELECT * FROM INFORMATION_SCHEMA.TRIGGERS
+  WHERE TRIGGER_SCHEMA="a@b" ORDER BY trigger_name;
+
+INSERT INTO `c@d` VALUES (2), (1);
+SELECT * FROM `c@d`;
+INSERT INTO t1 VALUES (3), (5);
+SELECT * FROM t1;
+
+DROP DATABASE `a@b`;
+
+USE test;
+
+--echo End of 5.1 tests
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@ -2230,6 +2230,7 @@ uint strconvert(CHARSET_INFO *from_cs, const char *from,
                CHARSET_INFO *to_cs, char *to, uint to_length, uint *errors);
 uint filename_to_tablename(const char *from, char *to, uint to_length);
 uint tablename_to_filename(const char *from, char *to, uint to_length);
+uint check_n_cut_mysql50_prefix(const char *from, char *to, uint to_length);
 #endif /* MYSQL_SERVER || INNODB_COMPATIBILITY_HOOKS */
 #ifdef MYSQL_SERVER
 uint build_table_filename(char *buff, size_t bufflen, const char *db,
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@ -114,6 +114,30 @@ uint filename_to_tablename(const char *from, char *to, uint to_length)
 }


+/**
+  Check if given string begins with "#mysql50#" prefix, cut it if so.
+  
+  @param   from          string to check and cut 
+  @param   to[out]       buffer for result string
+  @param   to_length     its size
+  
+  @retval
+    0      no prefix found
+  @retval
+    non-0  result string length
+*/
+
+uint check_n_cut_mysql50_prefix(const char *from, char *to, uint to_length)
+{
+  if (from[0] == '#' && 
+      !strncmp(from, MYSQL50_TABLE_NAME_PREFIX,
+               MYSQL50_TABLE_NAME_PREFIX_LENGTH))
+    return (uint) (strmake(to, from + MYSQL50_TABLE_NAME_PREFIX_LENGTH,
+                           to_length - 1) - to);
+  return 0;
+}
+
+
 /*
  Translate a table name to a file name (WL #1324).

@ -133,11 +157,8 @@ uint tablename_to_filename(const char *from, char *to, uint to_length)
  DBUG_ENTER("tablename_to_filename");
  DBUG_PRINT("enter", ("from '%s'", from));

-  if (from[0] == '#' && !strncmp(from, MYSQL50_TABLE_NAME_PREFIX,
-                                 MYSQL50_TABLE_NAME_PREFIX_LENGTH))
-    DBUG_RETURN((uint) (strmake(to, from+MYSQL50_TABLE_NAME_PREFIX_LENGTH,
-                                to_length-1) -
-                        (from + MYSQL50_TABLE_NAME_PREFIX_LENGTH)));
+  if ((length= check_n_cut_mysql50_prefix(from, to, to_length)))
+    DBUG_RETURN(length);
  length= strconvert(system_charset_info, from,
                     &my_charset_filename, to, to_length, &errors);
  if (check_if_legal_tablename(to) &&
--- a/sql/sql_trigger.cc
+++ b/sql/sql_trigger.cc
@ -1368,15 +1368,27 @@ bool Table_triggers_list::check_n_load(THD *thd, const char *db,

        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`"
+          or "ALTER DATABASE `#mysql50#somename` UPGRADE DATA DIRECTORY NAME"
+          we might be given table or database name with "#mysql50#" prefix (and
+          trigger's definiton contains un-prefixed version of the same name).
+          To remove this prefix we use check_n_cut_mysql50_prefix().
        */
-        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) ||
+                     (check_n_cut_mysql50_prefix(db, fname, sizeof(fname)) &&
+                      !my_strcasecmp(table_alias_charset, lex.query_tables->db, fname))) &&
+                    (!my_strcasecmp(table_alias_charset, lex.query_tables->table_name,
+                                    table_name) ||
+                     (check_n_cut_mysql50_prefix(table_name, fname, sizeof(fname)) &&
+                      !my_strcasecmp(table_alias_charset, lex.query_tables->table_name, fname))));
+#endif
        if (names_only)
        {
          lex_end(&lex);
@ -1692,7 +1704,8 @@ end:
  (change name of table in triggers' definitions).

  @param thd                 Thread context
-  @param db_name             Database of subject table
+  @param old_db_name         Old database of subject table
+  @param new_db_name         New database of subject table
  @param old_table_name      Old subject table's name
  @param new_table_name      New subject table's name

@ -1704,7 +1717,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 +1771,11 @@ Table_triggers_list::change_table_name_in_triggers(THD *thd,
  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;
@ -1772,7 +1786,8 @@ Table_triggers_list::change_table_name_in_triggers(THD *thd,
  Iterate though Table_triggers_list::names_list list and update
  .TRN files after renaming triggers' subject table.

-  @param db_name             Database of subject table
+  @param old_db_name         Old database of subject table
+  @param new_db_name         New database of subject table
  @param new_table_name      New subject table's name
  @param stopper             Pointer to Table_triggers_list::names_list at
                             which we should stop updating.
@ -1785,7 +1800,8 @@ Table_triggers_list::change_table_name_in_triggers(THD *thd,
 */

 LEX_STRING*
-Table_triggers_list::change_table_name_in_trignames(const char *db_name,
+Table_triggers_list::change_table_name_in_trignames(const char *old_db_name,
+                                                    const char *new_db_name,
                                                    LEX_STRING *new_table_name,
                                                    LEX_STRING *stopper)
 {
@ -1798,7 +1814,7 @@ Table_triggers_list::change_table_name_in_trignames(const char *db_name,
  while ((trigger= it_name++) != stopper)
  {
    trigname_file.length= build_table_filename(trigname_buff, FN_REFLEN-1,
-                                               db_name, trigger->str,
+                                               new_db_name, trigger->str,
                                               TRN_EXT, 0);
    trigname_file.str= trigname_buff;

@ -1807,6 +1823,16 @@ Table_triggers_list::change_table_name_in_trignames(const char *db_name,
    if (sql_create_definition_file(NULL, &trigname_file, &trigname_file_type,
                                   (uchar*)&trigname, trigname_file_parameters))
      return trigger;
+      
+    /* Remove stale .TRN file in case of database upgrade */
+    if (old_db_name)
+    {
+      if (rm_trigname_file(trigname_buff, old_db_name, trigger->str))
+      {
+        (void) rm_trigname_file(trigname_buff, new_db_name, trigger->str);
+        return trigger;
+      }
+    }
  }

  return 0;
@ -1840,6 +1866,7 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db,
 {
  TABLE table;
  bool result= 0;
+  bool upgrading50to51= FALSE; 
  LEX_STRING *err_trigname;
  DBUG_ENTER("change_table_name");

@ -1877,14 +1904,27 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db,
      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 check_n_cut_mysql50_prefix().
    */
    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];
+      if (check_n_cut_mysql50_prefix(db, dbname, sizeof(dbname)) && 
+          !my_strcasecmp(table_alias_charset, dbname, new_db))
+      {
+        upgrading50to51= TRUE;
+      }
+      else
+      {
+        my_error(ER_TRG_IN_WRONG_SCHEMA, MYF(0));
+        result= 1;
+        goto end;
+      }
    }
-    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,8 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db,
      goto end;
    }
    if ((err_trigname= table.triggers->change_table_name_in_trignames(
-                                         db, &new_table_name, 0)))
+                                         upgrading50to51 ? db : NULL,
+                                         new_db, &new_table_name, 0)))
    {
      /*
        If we were unable to update one of .TRN files properly we will
@ -1900,16 +1941,17 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db,
        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,
-                                                            &old_table_name,
-                                                            err_trigname);
-      (void) table.triggers->change_table_name_in_triggers(thd, db,
-                                                           &new_table_name,
-                                                           &old_table_name);
+      (void) table.triggers->change_table_name_in_trignames(
+                               upgrading50to51 ? new_db : NULL, db,
+                               &old_table_name, err_trigname);
+      (void) table.triggers->change_table_name_in_triggers(
+                               thd, db, new_db,
+                               &new_table_name, &old_table_name);
      result= 1;
      goto end;
    }
  }
+  
 end:
  delete table.triggers;
  free_root(&table.mem_root, MYF(0));
--- a/sql/sql_trigger.h
+++ b/sql/sql_trigger.h
@ -146,11 +146,13 @@ public:

 private:
  bool prepare_record1_accessors(TABLE *table);
-  LEX_STRING* change_table_name_in_trignames(const char *db_name,
+  LEX_STRING* change_table_name_in_trignames(const char *old_db_name,
+                                             const char *new_db_name,
                                             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);
 };