bzr commit into mysql-trunk branch (Dmitry.Lenev:3534) Bug#11746602

[Dmitry Lenev]

#At file:///home/dlenev/src/bzr/mysql-trunk-bug27480-3/ based on revid:dmitry.lenev@stripped

 3534 Dmitry Lenev	2011-04-01
      One more follow-up for the patch for Bug#11746602
      27480: Extend CREATE TEMPORARY TABLES privilege to
      allow temp table operations).
       
      Changed code not to produce unwarranted ER_CANT_REOPEN errors 
      in cases when prepared statement which used some table twice, 
      directly and indirectly, was re-executed in presence of 
      shadowing temporary table. Now, as expected, such statements 
      are re-prepared after validation in these cases.
       
      Adjusted code checking privileges for tables from UNION
      clauses in CREATE TABLE and ALTER TABLE to make these
      statements re-execution safe.
     @ mysql-test/r/ps_ddl.result
        Added tests which show: a) why it is important for
        open_temporary_tables() not to pre-open tables from
        statement's prelocking list; b) why it is necessary
        to reset table list elements from UNION clause of
        CREATE and ALTER TABLE statements.
     @ mysql-test/t/ps_ddl.test
        Added tests which show: a) why it is important for
        open_temporary_tables() not to pre-open tables from
        statement's prelocking list; b) why it is necessary
        to reset table list elements from UNION clause of
        CREATE and ALTER TABLE statements.
     @ sql/sql_alter.cc
        To make ALTER TABLE with UNION clause safe for re-execution
        we need to reset TABLE_LIST::table pointers for tables from
        this clause.
     @ sql/sql_base.cc
        - Changed open_temporary_tables() not to pre-open tables from
          the statements prelocking list. Old behavior led to 
          unwarranted ER_CANT_REOPEN errors in cases when prepared 
          statement which used some table twice, directly and 
          indirectly, was re-executed in presence of shadowing 
          temporary table. Now, as expected, such statements are 
          re-prepared after validation in these cases.
        - Adjusted open_temporary_tables() not to skip TABLE_LIST
          elements with non-zero 'table' pointer. This was not
          really necessary and new behavior allows to catch
          situations in which TABLE_LIST elements were not properly
          reset before statement re-execution or between two
          consecutive calls to this function.
     @ sql/sql_parse.cc
        To make CREATE TABLE with UNION clause safe for re-execution
        we need to reset TABLE_LIST::table pointers for tables from
        this clause.

    modified:
      mysql-test/r/ps_ddl.result
      mysql-test/t/ps_ddl.test
      sql/sql_alter.cc
      sql/sql_base.cc
      sql/sql_parse.cc
=== modified file 'mysql-test/r/ps_ddl.result'
--- a/mysql-test/r/ps_ddl.result	2010-08-18 09:35:41 +0000
+++ b/mysql-test/r/ps_ddl.result	2011-04-01 18:08:48 +0000
@@ -2316,3 +2316,43 @@ drop procedure if exists p_verify_reprep
 drop procedure if exists p1;
 drop function if exists f1;
 drop view if exists v1, v2;
+#
+# Additional coverage for refactoring which was made as part of work
+# on bug '27480: Extend CREATE TEMPORARY TABLES privilege to allow
+# temp table operations'. 
+#
+# Check that we don't try to pre-open temporary tables for the elements
+# from prelocking list, as this can lead to unwarranted ER_CANT_REOPEN
+# errors.
+DROP TABLE IF EXISTS t1, tm;
+CREATE TABLE t1 (a INT);
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+SET @a:= (SELECT COUNT(*) FROM t1);
+# Prelocking list for the below statement should
+# contain t1 twice - once for the INSERT and once
+# SELECT from the trigger.
+PREPARE stmt1 FROM 'INSERT INTO t1 VALUES (1)';
+EXECUTE stmt1;
+# Create temporary table which will shadow t1.
+CREATE TEMPORARY TABLE t1 (b int);
+# The below execution of statement should not fail with ER_CANT_REOPEN
+# error. Instead stmt1 should be auto-matically reprepared and succeed.
+EXECUTE stmt1;
+DEALLOCATE PREPARE stmt1;
+DROP TEMPORARY TABLE t1;
+DROP TABLE t1;
+#
+# Also check that we properly reset table list elements from UNION
+# clause of CREATE TABLE and ALTER TABLE statements.
+#
+CREATE TEMPORARY TABLE t1 (i INT);
+PREPARE stmt2 FROM 'CREATE TEMPORARY TABLE tm (i INT) ENGINE=MERGE UNION=(t1)';
+EXECUTE stmt2;
+DROP TEMPORARY TABLE tm;
+EXECUTE stmt2;
+DEALLOCATE PREPARE stmt2;
+PREPARE stmt3 FROM 'ALTER TABLE tm UNION=(t1)';
+EXECUTE stmt3;
+EXECUTE stmt3;
+DEALLOCATE PREPARE stmt3;
+DROP TEMPORARY TABLES tm, t1;

=== modified file 'mysql-test/t/ps_ddl.test'
--- a/mysql-test/t/ps_ddl.test	2010-02-06 10:28:06 +0000
+++ b/mysql-test/t/ps_ddl.test	2011-04-01 18:08:48 +0000
@@ -2098,3 +2098,47 @@ drop procedure if exists p1;
 drop function if exists f1;
 drop view if exists v1, v2;
 --enable_warnings
+
+
+--echo #
+--echo # Additional coverage for refactoring which was made as part of work
+--echo # on bug '27480: Extend CREATE TEMPORARY TABLES privilege to allow
+--echo # temp table operations'. 
+--echo #
+--echo # Check that we don't try to pre-open temporary tables for the elements
+--echo # from prelocking list, as this can lead to unwarranted ER_CANT_REOPEN
+--echo # errors.
+--disable_warnings
+DROP TABLE IF EXISTS t1, tm;
+--enable_warnings
+CREATE TABLE t1 (a INT);
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+  SET @a:= (SELECT COUNT(*) FROM t1);
+--echo # Prelocking list for the below statement should
+--echo # contain t1 twice - once for the INSERT and once
+--echo # SELECT from the trigger.
+PREPARE stmt1 FROM 'INSERT INTO t1 VALUES (1)';
+EXECUTE stmt1;
+--echo # Create temporary table which will shadow t1.
+CREATE TEMPORARY TABLE t1 (b int);
+--echo # The below execution of statement should not fail with ER_CANT_REOPEN
+--echo # error. Instead stmt1 should be auto-matically reprepared and succeed.
+EXECUTE stmt1;
+DEALLOCATE PREPARE stmt1;
+DROP TEMPORARY TABLE t1;
+DROP TABLE t1;
+--echo #
+--echo # Also check that we properly reset table list elements from UNION
+--echo # clause of CREATE TABLE and ALTER TABLE statements.
+--echo #
+CREATE TEMPORARY TABLE t1 (i INT);
+PREPARE stmt2 FROM 'CREATE TEMPORARY TABLE tm (i INT) ENGINE=MERGE UNION=(t1)';
+EXECUTE stmt2;
+DROP TEMPORARY TABLE tm;
+EXECUTE stmt2;
+DEALLOCATE PREPARE stmt2;
+PREPARE stmt3 FROM 'ALTER TABLE tm UNION=(t1)';
+EXECUTE stmt3;
+EXECUTE stmt3;
+DEALLOCATE PREPARE stmt3;
+DROP TEMPORARY TABLES tm, t1;

=== modified file 'sql/sql_alter.cc'
--- a/sql/sql_alter.cc	2011-03-26 10:56:27 +0000
+++ b/sql/sql_alter.cc	2011-04-01 18:08:48 +0000
@@ -67,6 +67,8 @@ bool Sql_cmd_alter_table::execute(THD *t
   /* If it is a merge table, check privileges for merge children. */
   if (create_info.merge_list.first)
   {
+    TABLE_LIST *tl;
+
     /* Pre-open underlying temporary tables to simplify privilege checking. */
     if (open_temporary_tables(thd, create_info.merge_list.first))
       DBUG_RETURN(TRUE);
@@ -84,7 +86,14 @@ bool Sql_cmd_alter_table::execute(THD *t
     */
     close_thread_tables(thd);
 
-    for (TABLE_LIST *tl= lex->query_tables; tl; tl= tl->next_global)
+    /*
+      To make things safe for re-execution and upcoming open_temporary_tables()
+      we need to reset TABLE_LIST::table pointers in both main table list and
+      and UNION clause.
+    */
+    for (tl= lex->query_tables; tl; tl= tl->next_global)
+      tl->table= NULL;
+    for (tl= lex->create_info.merge_list.first; tl; tl= tl->next_global)
       tl->table= NULL;
 
     if (open_temporary_tables(thd, lex->query_tables))

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2011-03-26 10:56:27 +0000
+++ b/sql/sql_base.cc	2011-04-01 18:08:48 +0000
@@ -6153,9 +6153,10 @@ bool open_temporary_table(THD *thd, TABL
 
 bool open_temporary_tables(THD *thd, TABLE_LIST *tl_list)
 {
+  TABLE_LIST *first_not_own= thd->lex->first_not_own_table();
   DBUG_ENTER("open_temporary_tables");
 
-  for (TABLE_LIST *tl= tl_list; tl; tl= tl->next_global)
+  for (TABLE_LIST *tl= tl_list; tl && tl != first_not_own; tl= tl->next_global)
   {
     if (tl->derived || tl->schema_table)
     {
@@ -6165,9 +6166,6 @@ bool open_temporary_tables(THD *thd, TAB
       continue;
     }
 
-    if (tl->table)
-      continue; // Skip if it's already opened.
-
     if (open_temporary_table(thd, tl))
       DBUG_RETURN(TRUE);
   }

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2011-03-28 11:49:45 +0000
+++ b/sql/sql_parse.cc	2011-04-01 18:08:48 +0000
@@ -7095,6 +7095,7 @@ bool create_table_precheck(THD *thd, TAB
   /* If it is a merge table, check privileges for merge children. */
   if (lex->create_info.merge_list.first)
   {
+    TABLE_LIST *tl;
     /*
       Pre-open temporary tables from UNION clause to simplify privilege
       checking for them.
@@ -7116,7 +7117,14 @@ bool create_table_precheck(THD *thd, TAB
     */
     close_thread_tables(thd);
 
-    for (TABLE_LIST *tl= lex->query_tables; tl; tl= tl->next_global)
+    /*
+      To make things safe for re-execution and upcoming open_temporary_tables()
+      we need to reset TABLE_LIST::table pointers in both main table list and
+      and UNION clause.
+    */
+    for (tl= lex->query_tables; tl; tl= tl->next_global)
+      tl->table= NULL;
+    for (tl= lex->create_info.merge_list.first; tl; tl= tl->next_global)
       tl->table= NULL;
 
     if (open_temporary_tables(thd, lex->query_tables))


Attachment: [text/bzr-bundle] bzr/[e-mail]