List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:March 7 2011 9:45am
Subject:bzr push into mysql-5.5 branch (jon.hauglid:3373 to 3374) Bug#11764779
View as plain text  
 3374 Jon Olav Hauglid	2011-03-07
      Bug #11764779 (former 57649)
      FLUSH TABLES under FLUSH TABLES <list> WITH READ LOCK leads 
      to assert failure.
      
      This assert was triggered if a statement tried up upgrade a metadata
      lock with an active FLUSH TABLE <list> WITH READ LOCK. The assert 
      checks that the connection already holds a global intention exclusive
      metadata lock. However, FLUSH TABLE <list> WITH READ LOCK does not
      acquire this lock in order to be compatible with FLUSH TABLES WITH
      READ LOCK. Therefore any metadata lock upgrade caused the assert to
      be triggered.
      
      This patch fixes the problem by preventing metadata lock upgrade
      if the connection has an active FLUSH TABLE <list> WITH READ LOCK.
      ER_TABLE_NOT_LOCKED_FOR_WRITE will instead be reported to the client.
      
      Test case added to flush.test.

    modified:
      mysql-test/r/flush.result
      mysql-test/t/flush.test
      sql/sql_base.cc
      sql/sql_base.h
      sql/sql_reload.cc
      sql/sql_table.cc
      sql/sql_trigger.cc
      sql/sql_truncate.cc
 3373 Alexander Barkov	2011-03-04
       Bug#11764503 (Bug#57341) Query in EXPLAIN EXTENDED shows wrong characters
      
        @ mysql-test/r/ctype_latin1.result
        @ mysql-test/r/ctype_utf8.result
        @ mysql-test/t/ctype_latin1.test
        @ mysql-test/t/ctype_utf8.test
        Adding tests
      
        @ sql/mysqld.h
        @ sql/item.cc
        @ sql/sql_parse.cc
        @ sql/sql_view.cc
      
        Refactoring (thanks to Guilhem for the idea):
      
        Item_string::print() was hard to understand because of the different
        QT_ constants: in "query_type==QT_x", QT_x is explicitely included
        but the other two QT_ are implicitely excluded. The combinations
        with '||' and '&&' make this even harder.
        - logic is now more "explicit" by changing QT_ constants to a bitmap of flags:
          QT_ORDINARY: no change,
          QT_IS -> QT_TO_SYSTEM_CHARSET | QT_WITHOUT_INTRODUCERS,
          QT_EXPLAIN -> QT_TO_SYSTEM_CHARSET
          (QT_EXPLAIN was introduced in the first version of the Bug#57341 patch)
        - Item_string::print() is rewritten using those flags
      
        Bugfix itself:
      
        When QT_TO_SYSTEM_CHARSET is used alone (with no QT_WITHOUT_INTRODUCERS),
        we print string literals as follows:
      
        - display introducers if they were in the original query
        - print ASCII characters as is
        - print non-ASCII characters using hex-escape
        Note: as "EXPLAIN" output is only for human readability purposes
        and does not need to be a pasrable SQL, so using hex-escape is Ok.
        ErrConvString class perfectly suites for hex escaping purposes.

    modified:
      mysql-test/r/ctype_latin1.result
      mysql-test/r/ctype_utf8.result
      mysql-test/t/ctype_latin1.test
      mysql-test/t/ctype_utf8.test
      sql/item.cc
      sql/mysqld.h
      sql/sql_parse.cc
      sql/sql_view.cc
=== modified file 'mysql-test/r/flush.result'
--- a/mysql-test/r/flush.result	2010-11-11 17:11:05 +0000
+++ b/mysql-test/r/flush.result	2011-03-07 09:08:10 +0000
@@ -451,3 +451,18 @@ unlock tables;
 handler t1 close;
 # Cleanup.
 drop tables t1, t2;
+#
+# Bug#57649 FLUSH TABLES under FLUSH TABLES <list> WITH READ LOCK leads
+#           to assert failure.
+#
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (a INT);
+FLUSH TABLES t1 WITH READ LOCK;
+FLUSH TABLES;
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET @a= 1;
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
+ALTER TABLE t1 COMMENT 'test';
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
+UNLOCK TABLES;
+DROP TABLE t1;

=== modified file 'mysql-test/t/flush.test'
--- a/mysql-test/t/flush.test	2010-11-11 17:11:05 +0000
+++ b/mysql-test/t/flush.test	2011-03-07 09:08:10 +0000
@@ -644,3 +644,27 @@ disconnect con2;
 --source include/wait_until_disconnected.inc
 connection default;
 drop tables t1, t2;
+
+
+--echo #
+--echo # Bug#57649 FLUSH TABLES under FLUSH TABLES <list> WITH READ LOCK leads
+--echo #           to assert failure.
+--echo #
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (a INT);
+FLUSH TABLES t1 WITH READ LOCK;
+
+# All these triggered the assertion
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
+FLUSH TABLES;
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET @a= 1;
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
+ALTER TABLE t1 COMMENT 'test';
+
+UNLOCK TABLES;
+DROP TABLE t1;

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2011-02-08 15:47:33 +0000
+++ b/sql/sql_base.cc	2011-03-07 09:08:10 +0000
@@ -1026,7 +1026,7 @@ bool close_cached_tables(THD *thd, TABLE
          table_list= table_list->next_global)
     {
       /* A check that the table was locked for write is done by the caller. */
-      TABLE *table= find_table_for_mdl_upgrade(thd->open_tables, table_list->db,
+      TABLE *table= find_table_for_mdl_upgrade(thd, table_list->db,
                                                table_list->table_name, TRUE);
 
       /* May return NULL if this table has already been closed via an alias. */
@@ -3120,22 +3120,26 @@ TABLE *find_locked_table(TABLE *list, co
    lock from the list of open tables, emit error if no such table
    found.
 
-   @param list       List of TABLE objects to be searched
+   @param thd        Thread context
    @param db         Database name.
    @param table_name Name of table.
    @param no_error   Don't emit error if no suitable TABLE
                      instance were found.
 
+   @note This function checks if the connection holds a global IX
+         metadata lock. If no such lock is found, it is not safe to
+         upgrade the lock and ER_TABLE_NOT_LOCKED_FOR_WRITE will be
+         reported.
+
    @return Pointer to TABLE instance with MDL_SHARED_NO_WRITE,
            MDL_SHARED_NO_READ_WRITE, or MDL_EXCLUSIVE metadata
            lock, NULL otherwise.
 */
 
-TABLE *find_table_for_mdl_upgrade(TABLE *list, const char *db,
-                                  const char *table_name,
-                                  bool no_error)
+TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db,
+                                  const char *table_name, bool no_error)
 {
-  TABLE *tab= find_locked_table(list, db, table_name);
+  TABLE *tab= find_locked_table(thd->open_tables, db, table_name);
 
   if (!tab)
   {
@@ -3143,19 +3147,29 @@ TABLE *find_table_for_mdl_upgrade(TABLE
       my_error(ER_TABLE_NOT_LOCKED, MYF(0), table_name);
     return NULL;
   }
-  else
+
+  /*
+    It is not safe to upgrade the metadata lock without a global IX lock.
+    This can happen with FLUSH TABLES <list> WITH READ LOCK as we in these
+    cases don't take a global IX lock in order to be compatible with
+    global read lock.
+  */
+  if (!thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "",
+                                      MDL_INTENTION_EXCLUSIVE))
   {
-    while (tab->mdl_ticket != NULL &&
-           !tab->mdl_ticket->is_upgradable_or_exclusive() &&
-           (tab= find_locked_table(tab->next, db, table_name)))
-      continue;
-    if (!tab)
-    {
-      if (!no_error)
-        my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name);
-      return 0;
-    }
+    if (!no_error)
+      my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name);
+    return NULL;
   }
+
+  while (tab->mdl_ticket != NULL &&
+         !tab->mdl_ticket->is_upgradable_or_exclusive() &&
+         (tab= find_locked_table(tab->next, db, table_name)))
+    continue;
+
+  if (!tab && !no_error)
+    my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name);
+
   return tab;
 }
 
@@ -4653,8 +4667,7 @@ open_tables_check_upgradable_mdl(THD *th
         Note that find_table_for_mdl_upgrade() will report an error if
         no suitable ticket is found.
       */
-      if (!find_table_for_mdl_upgrade(thd->open_tables, table->db,
-                                      table->table_name, FALSE))
+      if (!find_table_for_mdl_upgrade(thd, table->db, table->table_name, false))
         return TRUE;
     }
   }

=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h	2010-11-11 17:11:05 +0000
+++ b/sql/sql_base.h	2011-03-07 09:08:10 +0000
@@ -1,4 +1,4 @@
-/* Copyright 2006-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc.
+/* Copyright (c) 2006, 2011, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -290,7 +290,7 @@ bool tdc_open_view(THD *thd, TABLE_LIST
                    char *cache_key, uint cache_key_length,
                    MEM_ROOT *mem_root, uint flags);
 void tdc_flush_unused_tables();
-TABLE *find_table_for_mdl_upgrade(TABLE *list, const char *db,
+TABLE *find_table_for_mdl_upgrade(THD *thd, const char *db,
                                   const char *table_name,
                                   bool no_error);
 void mark_tmp_table_for_reuse(TABLE *table);

=== modified file 'sql/sql_reload.cc'
--- a/sql/sql_reload.cc	2010-12-07 16:11:13 +0000
+++ b/sql/sql_reload.cc	2011-03-07 09:08:10 +0000
@@ -1,4 +1,4 @@
-/* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -220,12 +220,26 @@ bool reload_acl_and_cache(THD *thd, unsi
         if (tables)
         {
           for (TABLE_LIST *t= tables; t; t= t->next_local)
-            if (!find_table_for_mdl_upgrade(thd->open_tables, t->db,
-                                            t->table_name, FALSE))
+            if (!find_table_for_mdl_upgrade(thd, t->db, t->table_name, false))
               return 1;
         }
         else
         {
+          /*
+            It is not safe to upgrade the metadata lock without GLOBAL IX lock.
+            This can happen with FLUSH TABLES <list> WITH READ LOCK as we in these
+            cases don't take a GLOBAL IX lock in order to be compatible with
+            global read lock.
+          */
+          if (thd->open_tables &&
+              !thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "",
+                                              MDL_INTENTION_EXCLUSIVE))
+          {
+            my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0),
+                     thd->open_tables->s->table_name.str);
+            return true;
+          }
+
           for (TABLE *tab= thd->open_tables; tab; tab= tab->next)
           {
             if (! tab->mdl_ticket->is_upgradable_or_exclusive())

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2011-01-26 13:23:29 +0000
+++ b/sql/sql_table.cc	2011-03-07 09:08:10 +0000
@@ -1917,7 +1917,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST
             by parser) it is safe to cache pointer to the TABLE instances
             in its elements.
           */
-          table->table= find_table_for_mdl_upgrade(thd->open_tables, table->db,
+          table->table= find_table_for_mdl_upgrade(thd, table->db,
                                                    table->table_name, false);
           if (!table->table)
             DBUG_RETURN(true);

=== modified file 'sql/sql_trigger.cc'
--- a/sql/sql_trigger.cc	2010-11-11 17:11:05 +0000
+++ b/sql/sql_trigger.cc	2011-03-07 09:08:10 +0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 2004-2005 MySQL AB, 2008-2009 Sun Microsystems, Inc
+/* Copyright (c) 2004, 2011, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -467,8 +467,7 @@ bool mysql_create_or_drop_trigger(THD *t
   if (thd->locked_tables_mode)
   {
     /* Under LOCK TABLES we must only accept write locked tables. */
-    if (!(tables->table= find_table_for_mdl_upgrade(thd->open_tables,
-                                                    tables->db,
+    if (!(tables->table= find_table_for_mdl_upgrade(thd, tables->db,
                                                     tables->table_name,
                                                     FALSE)))
       goto end;

=== modified file 'sql/sql_truncate.cc'
--- a/sql/sql_truncate.cc	2010-10-29 14:10:53 +0000
+++ b/sql/sql_truncate.cc	2011-03-07 09:08:10 +0000
@@ -1,4 +1,4 @@
-/* Copyright (c) 2010, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2010, 2011, Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -327,7 +327,7 @@ bool Truncate_statement::lock_table(THD
   */
   if (thd->locked_tables_mode)
   {
-    if (!(table= find_table_for_mdl_upgrade(thd->open_tables, table_ref->db,
+    if (!(table= find_table_for_mdl_upgrade(thd, table_ref->db,
                                             table_ref->table_name, FALSE)))
       DBUG_RETURN(TRUE);
 

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.5 branch (jon.hauglid:3373 to 3374) Bug#11764779Jon Olav Hauglid7 Mar