List:Commits« Previous MessageNext Message »
From:dlenev Date:April 29 2008 8:30am
Subject:bk commit into 6.1 tree (dlenev:1.2612) BUG#35529
View as plain text  
Below is the list of changes that have just been committed into a local
6.1 repository of dlenev.  When dlenev does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-04-29 12:29:51+04:00, dlenev@stripped +7 -0
  Fix for bug #35529 "Foreign keys: crash dropping partitioned
  referenced table".
  
  In --foreign-key-all-engines mode attempt to drop paritioned table with
  foreign keys led to weird error messages and even crashes.
  
  The problem was that open_table_def() table which we have used to
  get list of names of foreign keys to be dropped along with table
  assumes that it can create handler object for table which definition
  it reads. For partitioning engine this step involves access to
  handler (.par) file and doesn't work if handler files are not
  around (i.e. if they already have been removed or missing), which
  is the case in code responsible for dropping tables. So attempt
  to call this function failed and misleading error message was
  emitted.
  
  This fix introduces fill_table_def_fk_list() function, which does not
  do full-blown table open and is used instead of open_table_def() to
  get list of foreign keys for table in code responsible for drop tables.

  mysql-test/r/foreign_key_all_engines_2.result@stripped, 2008-04-29 12:29:43+04:00, dlenev@stripped +5 -0
    Added test case for bug #35529 "Foreign keys: crash dropping partitioned
    referenced table".

  mysql-test/t/foreign_key_all_engines_2.test@stripped, 2008-04-29 12:29:43+04:00, dlenev@stripped +20 -0
    Added test case for bug #35529 "Foreign keys: crash dropping partitioned
    referenced table".

  sql/fk_dd.cc@stripped, 2008-04-29 12:29:43+04:00, dlenev@stripped +6 -5
    fk_drop_all_constraint_names_for_table():
      Use fill_table_def_fk_list() instead of open_table_def() to get list
      of foreign keys for table. The problem with latter that for at least
      one engine (ha_partition) it can't be called if handler files for
      table were already deleted or are missing.

  sql/mysql_priv.h@stripped, 2008-04-29 12:29:43+04:00, dlenev@stripped +1 -0
    Added fill_table_def_fk_list() function which allows to get list of
    foreign keys for table without doing full-blown table open.

  sql/sql_table.cc@stripped, 2008-04-29 12:29:43+04:00, dlenev@stripped +15 -1
    mysql_rm_table_part2():
      Don't try to drop foreign keys names for internal temporary tables.
      They will be dropped when normal table with which they are associated
      is dropped.

  sql/table.cc@stripped, 2008-04-29 12:29:43+04:00, dlenev@stripped +214 -0
    Added fill_table_def_fk_list() function which allows to get list of
    foreign keys for table without doing full-blown table open. Also
    unlike open_table_def()/open_binary_frm() it does not create handler
    object for table being opened and therefore can be called for tables
    with missing handler files (e.g. .par file in case of ha_partition).

  sql/unireg.h@stripped, 2008-04-29 12:29:44+04:00, dlenev@stripped +1 -0
    Added constant for version of server starting from which foreign key
    definitions can appear in .FRM file

diff -Nrup a/mysql-test/r/foreign_key_all_engines_2.result b/mysql-test/r/foreign_key_all_engines_2.result
--- a/mysql-test/r/foreign_key_all_engines_2.result	2008-04-28 14:25:35 +04:00
+++ b/mysql-test/r/foreign_key_all_engines_2.result	2008-04-29 12:29:43 +04:00
@@ -4,3 +4,8 @@ create table t2 (s1 int, foreign key (s1
 drop table t2;
 create table t2 (s1 int references t1 (s1)) engine=innodb;
 drop table t1, t2;
+drop tables if exists t1, t2;
+create table t1 (s1 int primary key);
+create table t2 (s1 int references t2 (s1))
+partition by range (s1) (partition p1 values less than (1));
+drop table t2, t1;
diff -Nrup a/mysql-test/t/foreign_key_all_engines_2.test b/mysql-test/t/foreign_key_all_engines_2.test
--- a/mysql-test/t/foreign_key_all_engines_2.test	2008-04-28 14:25:35 +04:00
+++ b/mysql-test/t/foreign_key_all_engines_2.test	2008-04-29 12:29:43 +04:00
@@ -4,6 +4,7 @@
 # non-trivial features or storage engines.
 #
 --source include/have_innodb.inc
+--source include/have_partition.inc
 
 
 #
@@ -20,3 +21,22 @@ create table t2 (s1 int, foreign key (s1
 drop table t2;
 create table t2 (s1 int references t1 (s1)) engine=innodb;
 drop table t1, t2;
+
+
+# 
+# Bug #35529 "Foreign keys: crash dropping partitioned referenced table"
+#
+# Attempt to drop paritioned table with foreign keys led to weird error
+# messages and later crashes.
+--disable_warnings
+drop tables if exists t1, t2;
+--enable_warnings
+create table t1 (s1 int primary key);
+# This statement might start to fail once we reach one of the later
+# milestones of WL#148 but should work again once WL#4246 "Foreign Keys &
+# Partitioning: Implement Storage Engine API for foreign key support" is
+# complete.
+create table t2 (s1 int references t2 (s1))
+                partition by range (s1) (partition p1 values less than (1));
+# Dropping table t2 should succed
+drop table t2, t1;
diff -Nrup a/sql/fk_dd.cc b/sql/fk_dd.cc
--- a/sql/fk_dd.cc	2008-04-28 14:33:41 +04:00
+++ b/sql/fk_dd.cc	2008-04-29 12:29:43 +04:00
@@ -766,14 +766,15 @@ bool fk_drop_all_constraint_names_for_ta
                             table->table_name, "", 0));
 
   /*
-    To get list of foreign keys we need get access table's share.
-    Instead of doing full-blown share creation and going through
-    table definition cache we use object allocated on the stack
-    and initialize as if we were opening temporary table.
+    To get list of foreign keys we need get access to table's share.
+    Instead of doing full-blown share creation and filling we use
+    object allocated on the stack, initialized as if we were opening
+    temporary table and routine which reads only part of .FRM which
+    holds foreign key descriptions.
   */
   init_tmp_table_share(thd, &share, table->db, 0, table->table_name, path);
 
-  if (open_table_def(thd, &share, 0))
+  if (fill_table_def_fk_list(thd, &share))
   {
     free_table_share(&share);
     return TRUE;
diff -Nrup a/sql/mysql_priv.h b/sql/mysql_priv.h
--- a/sql/mysql_priv.h	2008-04-28 14:25:35 +04:00
+++ b/sql/mysql_priv.h	2008-04-29 12:29:43 +04:00
@@ -2172,6 +2172,7 @@ void init_tmp_table_share(THD *thd, TABL
 void free_table_share(TABLE_SHARE *share);
 int open_table_def(THD *thd, TABLE_SHARE *share, uint db_flags);
 void open_table_error(TABLE_SHARE *share, int error, int db_errno, int errarg);
+bool fill_table_def_fk_list(THD *thd, TABLE_SHARE *share);
 int open_table_from_share(THD *thd, TABLE_SHARE *share, const char *alias,
                           uint db_stat, uint prgflag, uint ha_open_flags,
                           TABLE *outparam, open_table_mode open_mode);
diff -Nrup a/sql/sql_table.cc b/sql/sql_table.cc
--- a/sql/sql_table.cc	2008-04-21 13:50:54 +04:00
+++ b/sql/sql_table.cc	2008-04-29 12:29:43 +04:00
@@ -1712,7 +1712,21 @@ int mysql_rm_table_part2(THD *thd, TABLE
       if (!error || error == ENOENT || error == HA_ERR_NO_SUCH_TABLE)
       {
         int new_error= 0;
-        if (opt_fk_all_engines)
+        /*
+          When mysql_rm_table_part2() is called as part of DROP DATABASE
+          statement it might be asked to remove some internal temporary
+          tables which were left from abnormally terminated ALTER TABLE
+          statements (for example, this can happen when server crashed
+          or was killed at them moment when ALTER was copying data from
+          old version of table to its new version with temporary name).
+          In such cases .FRM for these internal temporary tables can
+          contain descriptions of foreign keys which were defined on
+          original table at the moment of ALTER. We should not try to
+          drop FKs names for these FKs since a) they might be gone away
+          long ago b) even if they exist they should be dropped when
+          normal table with which they are associated is dropped.
+        */
+        if (opt_fk_all_engines && !table->internal_tmp_table)
           new_error= fk_drop_all_constraint_names_for_table(thd, table);
 
         if (!new_error)
diff -Nrup a/sql/table.cc b/sql/table.cc
--- a/sql/table.cc	2008-04-18 23:00:50 +04:00
+++ b/sql/table.cc	2008-04-29 12:29:43 +04:00
@@ -1667,6 +1667,220 @@ static int open_binary_frm(THD *thd, TAB
 } /* open_binary_frm */
 
 
+/**
+   Initialize foreign key list in TABLE_SHARE from .FRM section describing
+   foreign keys.
+
+   @param thd    Thread context.
+   @param share  TABLE_SHARE object to be initialized.
+
+   @note This is auxiliary function which allows to get information
+         about table's foreign keys without doing full-blown .FRM reading.
+         It might go away on one of the later milestones of WL#148.
+         If this won't happen then long-term it is going to be replaced
+         with function getting list of foreign keys directly from data
+         dictionary tables and not .FRMs.
+   @note Unlike open_table_def()/open_binary_frm() it does not create
+         handler object in the process, so one can safely call this
+         it even if handler files have been already deleted.
+
+   @retval FALSE  Success.
+   @retval TRUE   Failure.
+*/
+
+bool fill_table_def_fk_list(THD *thd, TABLE_SHARE *share)
+{
+  File file;
+  char	path[FN_REFLEN];
+  uchar head[64], forminfo[288], *buff= 0, *strpos;
+  uint mysql_version;
+  uint i, keys, keys_with_parser, key_flags, key_parts;
+  ulong form_pos, extra_offset, extra_length;
+  bool has_long_table_comment;
+  bool result= TRUE;
+
+  strxmov(path, share->normalized_path.str, reg_ext, NullS);
+  if ((file= my_open(path, O_RDONLY | O_SHARE, MYF(MY_WME))) < 0)
+    return TRUE;
+
+  /* Read .FRM header and check that it has proper version and type. */
+  if (my_read(file, head, 64, MYF(MY_NABP)))
+    goto err;
+
+  if (head[0] == (uchar) 254 && head[1] == 1)
+  {
+    if (!(head[2] == FRM_VER || head[2] == FRM_VER+1 ||
+          (head[2] >= FRM_VER+3 && head[2] <= FRM_VER+4)))
+    {
+      /* Unknown .FRM version. */
+      goto err;
+    }
+    /* Normal .FRM */
+  }
+  else if (memcmp(head, STRING_WITH_LEN("TYPE=")) == 0)
+  {
+    if (memcmp(head+5,"VIEW",4) == 0)
+    {
+      /* View can't have foreign keys so we can return immediately. */
+      goto ok;
+    }
+    goto err;
+  }
+  else
+  {
+    /* Something completely unintelligible. */
+    goto err;
+  }
+
+  mysql_version= uint4korr(head+51);
+  if (mysql_version < MYSQL_VERSION_FOREIGN_KEYS_IN_FRM)
+  {
+    /*
+      This is .FRM file from one of older versions of server,
+      so it can't contain information about foreign keys.
+    */
+    goto ok;
+  }
+  /*
+    Recent version of the server also means that version of .FRM format
+    is fairly recent, so parsing of .FRM file can be simplified.
+  */
+  DBUG_ASSERT(head[2] >= FRM_VER + 3);
+
+  /*
+    To be able correctly read information in extra data segment,
+    where info about foreign keys resides, we need need to know
+    if this table has long table comment.
+  */
+  if (!(form_pos= get_form_pos(file, head, (TYPELIB*) 0)))
+    goto err;
+  if (my_pread(file, forminfo, 288, form_pos, MYF(MY_NABP)))
+    goto err;
+  has_long_table_comment= (forminfo[46] == (uchar)255);
+
+  /*
+    Also we need to find out number of full-text keys that use pluggable
+    parsers. To do this we have to read a bit of description of each key.
+  */
+  VOID(my_seek(file, (ulong) uint2korr(head+6), MY_SEEK_SET, MYF(0)));
+  if (read_string(file, (uchar**) &buff, uint2korr(head+28)))
+    goto err;
+
+  if (buff[0] & 0x80)
+    keys= (buff[1] << 7) | (buff[0] & 0x7f);
+  else
+    keys= buff[0];
+
+  strpos= buff + 6;
+  keys_with_parser= 0;
+
+  for (i=0 ; i < keys ; i++)
+  {
+    key_flags= (uint) uint2korr(strpos) ^ HA_NOSAME;
+    key_parts= (uint) strpos[4];
+    strpos+= 8 + key_parts * 9;
+
+    if (key_flags & HA_USES_PARSER)
+      keys_with_parser++;
+  }
+  my_free(buff, MYF(0));
+  buff= 0;
+  /* End of work with key descriptions. */
+
+  /* Now we can work with extra data segment. */
+  extra_offset= (ulong) (uint2korr(head+6) +
+                         ((uint2korr(head+14) == 0xffff ?
+                           uint4korr(head+47) : uint2korr(head+14))) +
+                         uint2korr(head+16));
+
+  if ((extra_length= uint4korr(head+55)))
+  {
+    uchar *next_chunk, *buff_end;
+    DBUG_PRINT("info", ("extra segment size is %lu bytes", extra_length));
+    if (!(next_chunk= buff= (uchar*) my_malloc(extra_length, MYF(MY_WME))))
+      goto err;
+    buff_end= buff + extra_length;
+
+    if (my_pread(file, buff, extra_length, extra_offset, MYF(MY_NABP)))
+      goto err;
+
+    /* Skip connect string. */
+    next_chunk+= uint2korr(next_chunk) + 2;
+
+    /* Skip the engine name. */
+    if (next_chunk + 2 < buff_end)
+      next_chunk+= uint2korr(next_chunk) + 2;
+
+    /* Skip partition info. */
+    if (next_chunk + 5 < buff_end)
+      next_chunk+= uint4korr(next_chunk) + 5;
+
+    /* Skip new auto_partitioned indicator, which was introduced in 5.1.11 */
+    DBUG_ASSERT (mysql_version >= 50110);
+    next_chunk++;
+
+    /* Skip names of parsers used by full text keys. */
+    for (i= 0; i < keys_with_parser; i++)
+    {
+      if (next_chunk >= buff_end)
+      {
+        DBUG_PRINT("error",
+                   ("fulltext key uses parser that is not defined in .frm"));
+        goto err;
+      }
+      next_chunk+= strlen((char*) next_chunk) + 1;
+    }
+
+    /* Skip long table comment. */
+    if (has_long_table_comment)
+    {
+      if (next_chunk + 2 > buff_end)
+      {
+          DBUG_PRINT("error",
+                     ("long table comment is not defined in .frm"));
+          goto err;
+      }
+      next_chunk+= 2 + uint2korr(next_chunk);
+    }
+
+    /* Skip section with column's COLUMN_FORMAT and STORAGE properties. */
+    DBUG_ASSERT (mysql_version >= MYSQL_VERSION_TABLESPACE_IN_FRM &&
+                 mysql_version >= MYSQL_VERSION_TABLESPACE_IN_FRM_CGE);
+    if (next_chunk >= buff_end)
+    {
+      DBUG_PRINT("error", ("Found no field extra info"));
+      goto err;
+    }
+    else
+    {
+      DBUG_PRINT("info", ("Found field extra info"));
+      next_chunk+= uint2korr(next_chunk);
+    }
+
+    if (fk_restore_from_frm_section(thd, share, (const char**)&next_chunk,
+                                    (const char*)buff_end))
+      goto err;
+
+    if (next_chunk > buff_end)
+    {
+      DBUG_PRINT("error", ("Buffer overflow in field extra info"));
+      goto err;
+    }
+  }
+
+ok:
+  result= FALSE;
+
+err:
+  if (result)
+    my_error(ER_NOT_FORM_FILE, MYF(0), path);
+  if (buff)
+    my_free(buff, MYF(0));
+  my_close(file, MYF(0));
+  return result;
+}
+
+
 /*
   Open a table based on a TABLE_SHARE
 
diff -Nrup a/sql/unireg.h b/sql/unireg.h
--- a/sql/unireg.h	2008-03-12 11:25:25 +03:00
+++ b/sql/unireg.h	2008-04-29 12:29:44 +04:00
@@ -202,6 +202,7 @@
 #define MYSQL_VERSION_TABLESPACE_IN_FRM_CGE 50120
 #define MYSQL_VERSION_TABLESPACE_IN_FRM 50205
 #define MYSQL_VERSION_TABLESPACE_IN_FRM_STR "50205"
+#define MYSQL_VERSION_FOREIGN_KEYS_IN_FRM 60100
 
 /*
   Minimum length pattern before Turbo Boyer-Moore is used
Thread
bk commit into 6.1 tree (dlenev:1.2612) BUG#35529dlenev29 Apr