List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:May 26 2011 3:50pm
Subject:bzr push into mysql-5.5 branch (Dmitry.Lenev:3410 to 3411) Bug#11762012
View as plain text  
 3411 Dmitry Lenev	2011-05-26
      Fix for bug #11762012 - "54553: INNODB ASSERTS IN
      HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK".
      
      Attempt to update an InnoDB temporary table under LOCK TABLES
      led to assertion failure in both debug and production builds
      if this temporary table was explicitly locked for READ. The
      same scenario works fine for MyISAM temporary tables.
      
      The assertion failure was caused by discrepancy between lock
      that was requested on the rows of temporary table at LOCK TABLES
      time and by update operation. Since SQL-layer requested a
      read-lock at LOCK TABLES time InnoDB engine assumed that upcoming
      statements which are going to be executed under LOCK TABLES will
      only read table and therefore should acquire only S-lock.
      An update operation broken this assumption by requesting X-lock.
      
      Possible approaches to fixing this problem are:
      
      1) Skip locking of temporary tables as locking doesn't make any
         sense for connection-local objects.
      2) Prohibit changing of temporary table locked by LOCK TABLES ...
         READ.
      
      Unfortunately both of these approaches have drawbacks which make
      them unviable for stable versions of server.
      
      So this patch takes another approach and changes code in such way
      that LOCK TABLES for a temporary table will always request write
      lock. In 5.5 version of this patch switch from read lock to write
      lock is done on SQL-layer.
     @ mysql-test/suite/innodb/r/innodb_mysql.result
        Added test for bug #11762012 - "54553: INNODB ASSERTS IN
        HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK".
     @ mysql-test/suite/innodb/t/innodb_mysql.test
        Added test for bug #11762012 - "54553: INNODB ASSERTS IN
        HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK".
     @ sql/sql_parse.cc
        Since a temporary table locked by LOCK TABLES can be updated even
        if it was only locked for read we always request TL_WRITE locks
        for such tables at LOCK TABLES time. This allows to avoid 
        discrepancy between locks acquired at LOCK TABLES time and by
        a statement executed under LOCK TABLES. Such a discrepancy has
        caused problems for InnoDB storage engine.
        
        To support this change a part of code implementing LOCK TABLES 
        has been moved to a helper function.

    modified:
      mysql-test/suite/innodb/r/innodb_mysql.result
      mysql-test/suite/innodb/t/innodb_mysql.test
      sql/sql_parse.cc
 3410 Dmitry Lenev	2011-05-26 [merge]
      Null-merged 5.1 version of fix for bug #11762012 - "54553:
      INNODB ASSERTS IN HA_INNOBASE::UPDATE_ROW, TEMPORARY TABLE,
      TABLE LOCK" into 5.5 tree. 5.5 version of fix will be
      committed and pushed separately.

=== modified file 'mysql-test/suite/innodb/r/innodb_mysql.result'
--- a/mysql-test/suite/innodb/r/innodb_mysql.result	2011-05-26 15:09:16 +0000
+++ b/mysql-test/suite/innodb/r/innodb_mysql.result	2011-05-26 15:50:06 +0000
@@ -2663,7 +2663,6 @@ COUNT(*)
 1537
 SET SESSION sort_buffer_size = DEFAULT;
 DROP TABLE t1;
-End of 5.1 tests
 #
 # Test for bug #39932 "create table fails if column for FK is in different
 #                      case than in corr index".
@@ -2685,6 +2684,23 @@ t2	CREATE TABLE `t2` (
 ) ENGINE=InnoDB DEFAULT CHARSET=latin1
 drop table t2, t1;
 #
+# Test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE::
+#                           UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK".
+#
+DROP TABLE IF EXISTS t1;
+CREATE TEMPORARY TABLE t1 (c int) ENGINE = InnoDB;
+INSERT INTO t1 VALUES (1);
+LOCK TABLES t1 READ;
+# Even though temporary table was locked for READ we
+# still allow writes to it to be compatible with MyISAM.
+# This is possible since due to fact that temporary tables
+# are specific to connection and therefore locking for them
+# is irrelevant.
+UPDATE t1 SET c = 5;
+UNLOCK TABLES;
+DROP TEMPORARY TABLE t1;
+End of 5.1 tests
+#
 # Bug#44613 SELECT statement inside FUNCTION takes a shared lock
 #
 DROP TABLE IF EXISTS t1;

=== modified file 'mysql-test/suite/innodb/t/innodb_mysql.test'
--- a/mysql-test/suite/innodb/t/innodb_mysql.test	2011-05-26 15:09:16 +0000
+++ b/mysql-test/suite/innodb/t/innodb_mysql.test	2011-05-26 15:50:06 +0000
@@ -830,8 +830,6 @@ SET SESSION sort_buffer_size = DEFAULT;
 
 DROP TABLE t1;
 
---echo End of 5.1 tests
-
 
 --echo #
 --echo # Test for bug #39932 "create table fails if column for FK is in different
@@ -852,6 +850,28 @@ drop table t2, t1;
 
 
 --echo #
+--echo # Test for bug #11762012 - "54553: INNODB ASSERTS IN HA_INNOBASE::
+--echo #                           UPDATE_ROW, TEMPORARY TABLE, TABLE LOCK".
+--echo #
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+CREATE TEMPORARY TABLE t1 (c int) ENGINE = InnoDB;
+INSERT INTO t1 VALUES (1);
+LOCK TABLES t1 READ;
+--echo # Even though temporary table was locked for READ we
+--echo # still allow writes to it to be compatible with MyISAM.
+--echo # This is possible since due to fact that temporary tables
+--echo # are specific to connection and therefore locking for them
+--echo # is irrelevant.
+UPDATE t1 SET c = 5;
+UNLOCK TABLES;
+DROP TEMPORARY TABLE t1;
+
+--echo End of 5.1 tests
+
+
+--echo #
 --echo # Bug#44613 SELECT statement inside FUNCTION takes a shared lock
 --echo #
 

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2011-05-21 08:21:08 +0000
+++ b/sql/sql_parse.cc	2011-05-26 15:50:06 +0000
@@ -1749,6 +1749,67 @@ bool sp_process_definer(THD *thd)
 
 
 /**
+  Auxiliary call that opens and locks tables for LOCK TABLES statement
+  and initializes the list of locked tables.
+
+  @param thd     Thread context.
+  @param tables  List of tables to be locked.
+
+  @return FALSE in case of success, TRUE in case of error.
+*/
+
+static bool lock_tables_open_and_lock_tables(THD *thd, TABLE_LIST *tables)
+{
+  Lock_tables_prelocking_strategy lock_tables_prelocking_strategy;
+  uint counter;
+  TABLE_LIST *table;
+
+  thd->in_lock_tables= 1;
+
+  if (open_tables(thd, &tables, &counter, 0, &lock_tables_prelocking_strategy))
+    goto err;
+
+  /*
+    We allow to change temporary tables even if they were locked for read
+    by LOCK TABLES. To avoid a discrepancy between lock acquired at LOCK
+    TABLES time and by the statement which is later executed under LOCK TABLES
+    we ensure that for temporary tables we always request a write lock (such
+    discrepancy can cause problems for the storage engine).
+    We don't set TABLE_LIST::lock_type in this case as this might result in
+    extra warnings from THD::decide_logging_format() even though binary logging
+    is totally irrelevant for LOCK TABLES.
+  */
+  for (table= tables; table; table= table->next_global)
+    if (!table->placeholder() && table->table->s->tmp_table)
+      table->table->reginfo.lock_type= TL_WRITE;
+
+  if (lock_tables(thd, tables, counter, 0) ||
+      thd->locked_tables_list.init_locked_tables(thd))
+    goto err;
+
+  thd->in_lock_tables= 0;
+
+  return FALSE;
+
+err:
+  thd->in_lock_tables= 0;
+
+  trans_rollback_stmt(thd);
+  /*
+    Need to end the current transaction, so the storage engine (InnoDB)
+    can free its locks if LOCK TABLES locked some tables before finding
+    that it can't lock a table in its list
+  */
+  trans_commit_implicit(thd);
+  /* Close tables and release metadata locks. */
+  close_thread_tables(thd);
+  DBUG_ASSERT(!thd->locked_tables_mode);
+  thd->mdl_context.release_transactional_locks();
+  return TRUE;
+}
+
+
+/**
   Execute command saved in thd and lex->sql_command.
 
   @param thd                       Thread handle
@@ -3093,31 +3154,11 @@ end_with_restore_list:
       goto error;
 
     thd->variables.option_bits|= OPTION_TABLE_LOCK;
-    thd->in_lock_tables=1;
 
-    {
-      Lock_tables_prelocking_strategy lock_tables_prelocking_strategy;
-
-      res= (open_and_lock_tables(thd, all_tables, FALSE, 0,
-                                 &lock_tables_prelocking_strategy) ||
-            thd->locked_tables_list.init_locked_tables(thd));
-    }
-
-    thd->in_lock_tables= 0;
+    res= lock_tables_open_and_lock_tables(thd, all_tables);
 
     if (res)
     {
-      trans_rollback_stmt(thd);
-      /*
-        Need to end the current transaction, so the storage engine (InnoDB)
-        can free its locks if LOCK TABLES locked some tables before finding
-        that it can't lock a table in its list
-      */
-      trans_commit_implicit(thd);
-      /* Close tables and release metadata locks. */
-      close_thread_tables(thd);
-      DBUG_ASSERT(!thd->locked_tables_mode);
-      thd->mdl_context.release_transactional_locks();
       thd->variables.option_bits&= ~(OPTION_TABLE_LOCK);
     }
     else

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.5 branch (Dmitry.Lenev:3410 to 3411) Bug#11762012Dmitry Lenev26 May