List:Commits« Previous MessageNext Message »
From:dlenev Date:April 22 2008 12:19pm
Subject:bk commit into 6.1 tree (dlenev:1.2607) 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-22 16:18:54+04:00, dlenev@stripped +7 -0
  Tentative 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.
  
  Question for reviewer is marked by QQ.

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

  mysql-test/r/foreign_key_all_engines_2.result@stripped, 2008-04-22 16:18:49+04:00, dlenev@stripped +0 -0

  mysql-test/t/foreign_key_all_engines_2-master.opt@stripped, 2008-04-22 16:18:49+04:00, dlenev@stripped +1 -0
    Added test case for bug #35529 "Foreign keys: crash dropping partitioned
    referenced table".

  mysql-test/t/foreign_key_all_engines_2-master.opt@stripped, 2008-04-22 16:18:49+04:00, dlenev@stripped +0 -0

  mysql-test/t/foreign_key_all_engines_2.test@stripped, 2008-04-22 16:18:49+04:00, dlenev@stripped +25 -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-22 16:18:49+04:00, dlenev@stripped +0 -0

  sql/fk_dd.cc@stripped, 2008-04-22 16:18:49+04:00, dlenev@stripped +14 -5
    fk_drop_all_constraint_names_for_table():
      Don't try to get list of foreign keys for internal temporary tables.
      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-22 16:18:49+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/table.cc@stripped, 2008-04-22 16:18:49+04:00, dlenev@stripped +220 -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-22 16:18:49+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
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/r/foreign_key_all_engines_2.result	2008-04-22 16:18:49 +04:00
@@ -0,0 +1,5 @@
+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-master.opt b/mysql-test/t/foreign_key_all_engines_2-master.opt
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/t/foreign_key_all_engines_2-master.opt	2008-04-22 16:18:49 +04:00
@@ -0,0 +1 @@
+--foreign-key-all-engines=1
diff -Nrup a/mysql-test/t/foreign_key_all_engines_2.test b/mysql-test/t/foreign_key_all_engines_2.test
--- /dev/null	Wed Dec 31 16:00:00 196900
+++ b/mysql-test/t/foreign_key_all_engines_2.test	2008-04-22 16:18:49 +04:00
@@ -0,0 +1,25 @@
+#
+# Additional test-coverage for the 2nd and other milestones of
+# WL#148 "Foreign keys". This file is for tests which depend on
+# non-trivial features or storage engines.
+#
+--source include/have_partition.inc
+
+
+# 
+# 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-18 15:49:29 +04:00
+++ b/sql/fk_dd.cc	2008-04-22 16:18:49 +04:00
@@ -761,18 +761,27 @@ bool fk_drop_all_constraint_names_for_ta
 
   safe_mutex_assert_owner(&LOCK_open);
 
+  /*
+    Even if this temporary table was created by ALTER as new version of
+    normal table we should not drop foreign key names associated with it
+    as they are also associated with old version of the table.
+  */
+  if (table->internal_tmp_table)
+    return FALSE;
+
   VOID(build_table_filename(path, FN_REFLEN - 1, table->db,
                             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-03-21 13:55:40 +03:00
+++ b/sql/mysql_priv.h	2008-04-22 16:18:49 +04:00
@@ -2171,6 +2171,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/table.cc b/sql/table.cc
--- a/sql/table.cc	2008-03-21 13:55:43 +03:00
+++ b/sql/table.cc	2008-04-22 16:18:49 +04:00
@@ -1667,6 +1667,226 @@ 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.
+
+   QQ: This is a fairly big piece of code which is to some extent
+       duplicates code in open_table_def() and open_binary_frm().
+       May be it is better to use them instead (this will require
+       changing them to be able to work even if handler files are
+       gone)?
+
+   @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-22 16:18:49 +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.2607) BUG#35529dlenev22 Apr
  • Re: bk commit into 6.1 tree (dlenev:1.2607) BUG#35529Konstantin Osipov28 Apr