List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:December 16 2010 9:55am
Subject:bzr commit into mysql-5.5-bugteam branch (jon.hauglid:3202) Bug#58730
View as plain text  
#At file:///export/home/x/mysql-5.5-bugteam-bug58730/ based on revid:jonathan.perkin@stripped

 3202 Jon Olav Hauglid	2010-12-16
      Bug #58730 Assertion failed: table->key_read == 0 in close_thread_table,
                 temptable views
      
      The TABLE::key_read field indicates if the optimizer has found that row
      retrieval only should access the index tree. The triggered assert
      inside close_thread_table() checks that this field has been reset when
      the table is about to be closed.
      
      During normal execution, these fields are reset right before tables are
      closed at the end of mysql_execute_command(). But in the case of errors,
      tables are closed earlier. The patch for Bug#52044 refactored the open
      tables code so that close_thread_tables() is called immediately if
      opening of tables fails. At this point in the execution, it could
      happend that all TABLE::key_read fields had not been properly reset,
      therefore triggering the assert.
      
      The problematic statement in this case was EXPLAIN where the query
      accessed two derived tables and where the first derived table was
      processed successfully while the second derived table was not.
      Since it was an EXPLAIN, TABLE::key_read fields were not reset after
      successful derived table processing since the state needs to be 
      accessible afterwards. When processing of the second derived table
      failed, it's corresponding SELECT_LEX_UNIT was cleaned, which caused
      it's TABLE::key_read fields to be reset. Since processing failed,
      the error path of open_and_lock_tables() was entered and
      close_thread_tables() was called. The assert was then triggered due
      to the TABLE::key_read fields set during processing of the first
      derived table.
      
      This patch fixes the problem by adding a new derived table processor,
      mysql_derived_cleanup() that is called after mysql_derived_filling().
      It causes cleanup of all SELECT_LEX_UNITs to be called, resetting
      all relevant TABLE::key_read fields.
      
      Test case added to derived.test.

    modified:
      mysql-test/r/derived.result
      mysql-test/t/derived.test
      sql/sql_base.cc
      sql/sql_derived.cc
      sql/sql_derived.h
      sql/sql_update.cc
=== modified file 'mysql-test/r/derived.result'
--- a/mysql-test/r/derived.result	2010-12-14 10:46:00 +0000
+++ b/mysql-test/r/derived.result	2010-12-16 09:55:23 +0000
@@ -412,3 +412,18 @@ MIN(i)
 1
 DROP TABLE t1;
 # End of 5.0 tests
+#
+# Bug#58730 Assertion failed: table->key_read == 0 in close_thread_table,
+#           temptable views
+#
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (b INT, KEY (b));
+INSERT INTO t1 VALUES (1),(1);
+INSERT INTO t2 VALUES (1),(1);
+CREATE algorithm=temptable VIEW v1 AS
+SELECT 1 FROM t1 LEFT JOIN t1 t3 ON 1 > (SELECT 1 FROM t1);
+CREATE algorithm=temptable VIEW v2 AS SELECT 1 FROM t2;
+EXPLAIN SELECT 1 FROM t1 JOIN v1 ON 1 > (SELECT 1 FROM v2);
+ERROR 21000: Subquery returns more than 1 row
+DROP TABLE t1, t2;
+DROP VIEW v1, v2;

=== modified file 'mysql-test/t/derived.test'
--- a/mysql-test/t/derived.test	2010-12-14 10:46:00 +0000
+++ b/mysql-test/t/derived.test	2010-12-16 09:55:23 +0000
@@ -313,3 +313,25 @@ WHERE j = SUBSTRING('12', (SELECT * FROM
 DROP TABLE t1;
 
 --echo # End of 5.0 tests
+
+
+--echo #
+--echo # Bug#58730 Assertion failed: table->key_read == 0 in close_thread_table,
+--echo #           temptable views
+--echo #
+
+CREATE TABLE t1 (a INT);
+CREATE TABLE t2 (b INT, KEY (b));
+INSERT INTO t1 VALUES (1),(1);
+INSERT INTO t2 VALUES (1),(1);
+
+CREATE algorithm=temptable VIEW v1 AS
+  SELECT 1 FROM t1 LEFT JOIN t1 t3 ON 1 > (SELECT 1 FROM t1);
+CREATE algorithm=temptable VIEW v2 AS SELECT 1 FROM t2;
+
+# This caused the assert to be triggered.
+--error ER_SUBQUERY_NO_1_ROW
+EXPLAIN SELECT 1 FROM t1 JOIN v1 ON 1 > (SELECT 1 FROM v2);
+
+DROP TABLE t1, t2;
+DROP VIEW v1, v2;

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2010-11-11 17:11:05 +0000
+++ b/sql/sql_base.cc	2010-12-16 09:55:23 +0000
@@ -5408,11 +5408,19 @@ bool open_and_lock_tables(THD *thd, TABL
   if (lock_tables(thd, tables, counter, flags))
     goto err;
 
-  if (derived &&
-      (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
-       (thd->fill_derived_tables() &&
-        mysql_handle_derived(thd->lex, &mysql_derived_filling))))
-    goto err;
+  if (derived)
+  {
+    if (mysql_handle_derived(thd->lex, &mysql_derived_prepare))
+      goto err;
+    if (thd->fill_derived_tables() &&
+        mysql_handle_derived(thd->lex, &mysql_derived_filling))
+    {
+      mysql_handle_derived(thd->lex, &mysql_derived_cleanup);
+      goto err;
+    }
+    if (!thd->lex->describe)
+      mysql_handle_derived(thd->lex, &mysql_derived_cleanup);
+  }
 
   DBUG_RETURN(FALSE);
 err:

=== modified file 'sql/sql_derived.cc'
--- a/sql/sql_derived.cc	2010-12-14 10:46:00 +0000
+++ b/sql/sql_derived.cc	2010-12-16 09:55:23 +0000
@@ -307,13 +307,21 @@ bool mysql_derived_filling(THD *thd, LEX
       */
       if (derived_result->flush())
         res= TRUE;
-
-      if (!lex->describe)
-        unit->cleanup();
     }
-    else
-      unit->cleanup();
     lex->current_select= save_current_select;
   }
   return res;
 }
+
+
+/**
+   Cleans up the SELECT_LEX_UNIT for the derived table (if any).
+*/
+
+bool mysql_derived_cleanup(THD *thd, LEX *lex, TABLE_LIST *derived)
+{
+  SELECT_LEX_UNIT *unit= derived->derived;
+  if (unit)
+    unit->cleanup();
+  return false;
+}

=== modified file 'sql/sql_derived.h'
--- a/sql/sql_derived.h	2010-04-12 13:17:37 +0000
+++ b/sql/sql_derived.h	2010-12-16 09:55:23 +0000
@@ -26,4 +26,16 @@ bool mysql_handle_derived(LEX *lex, bool
 bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *t);
 bool mysql_derived_filling(THD *thd, LEX *lex, TABLE_LIST *t);
 
+/**
+   Cleans up the SELECT_LEX_UNIT for the derived table (if any).
+
+   @param  thd         Thread handler
+   @param  lex         LEX for this thread
+   @param  derived     TABLE_LIST for the derived table
+
+   @retval  false  Success
+   @retval  true   Failure
+*/
+bool mysql_derived_cleanup(THD *thd, LEX *lex, TABLE_LIST *derived);
+
 #endif /* SQL_DERIVED_INCLUDED */

=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc	2010-11-16 09:05:19 +0000
+++ b/sql/sql_update.cc	2010-12-16 09:55:23 +0000
@@ -296,11 +296,17 @@ int mysql_update(THD *thd,
   if (lock_tables(thd, table_list, table_count, 0))
     DBUG_RETURN(1);
 
-  if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
-      (thd->fill_derived_tables() &&
-       mysql_handle_derived(thd->lex, &mysql_derived_filling)))
+  if (mysql_handle_derived(thd->lex, &mysql_derived_prepare))
     DBUG_RETURN(1);
 
+  if (thd->fill_derived_tables() &&
+      mysql_handle_derived(thd->lex, &mysql_derived_filling))
+  {
+    mysql_handle_derived(thd->lex, &mysql_derived_cleanup);
+    DBUG_RETURN(1);
+  }
+  mysql_handle_derived(thd->lex, &mysql_derived_cleanup);
+
   thd_proc_info(thd, "init");
   table= table_list->table;
 
@@ -1194,7 +1200,11 @@ int mysql_multi_update_prepare(THD *thd)
  
   if (thd->fill_derived_tables() &&
       mysql_handle_derived(lex, &mysql_derived_filling))
+  {
+    mysql_handle_derived(lex, &mysql_derived_cleanup);
     DBUG_RETURN(TRUE);
+  }
+  mysql_handle_derived(lex, &mysql_derived_cleanup);
 
   DBUG_RETURN (FALSE);
 }


Attachment: [text/bzr-bundle] bzr/jon.hauglid@oracle.com-20101216095523-uguxqyzv0aai9wkb.bundle
Thread
bzr commit into mysql-5.5-bugteam branch (jon.hauglid:3202) Bug#58730Jon Olav Hauglid16 Dec