List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:April 1 2011 6:09pm
Subject:bzr push into mysql-trunk branch (Dmitry.Lenev:3533 to 3534) Bug#11746602
View as plain text  
 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
 3533 Dmitry Lenev	2011-03-28
      A follow-up for the patch for Bug#11746602 (27480: Extend 
      CREATE TEMPORARY TABLES privilege to allow temp table 
      operations).
      
      After main patch for this bug additional check for privileges
      required for SHOW statements might require opening and closing
      of temporary tables. Since doing this from within 
      check_table_access() function looks like a dangerous thing,
      the current patch moves this additional check outside of
      this function. To support this change it also removes some 
      duplicated code.
     @ sql/sql_parse.cc
        - Moved code responsible for the first stage of checking
          privileges for SELECT statement (global, db and table-level
          privileges) to a separate function - select_precheck().
          Adjusted code for select-like statements to use this
          function.
        - Got rid of duplicate code handling SHOW EVENTS and SHOW
          PROCEDURE/FUNCTION STATUS. As a side-effect of this change
          now these statements reset last_query_cost status variable
          like most of other SHOW statements.
        - Moved code which performs additional check for privileges
          required to perform SHOW statement from check_table_access()
          function to newly created select_precheck() function. Doing
          this privilege check, which might require opening and closing
          of temporary tables, inside of check_table_access() looks
          like a dangerous thing.
        - Got rid of redundant code in check_show_access() which
          automatically granted SELECT_ACL on I_S table for SHOW
          statement. This is already done for all I_S tables in
          IS_internal_schema_access::check() member function.
     @ sql/sql_parse.h
        Introduced select_precheck() function which performs first
        stage of privilege checking for SELECT statements.
     @ sql/sql_prepare.cc
        Code responsible for the first stage of checking privileges
        for SELECT statements (global, db and table-level privileges)
        has been moved to new function select_precheck().

    modified:
      sql/sql_parse.cc
      sql/sql_parse.h
      sql/sql_prepare.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))

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (Dmitry.Lenev:3533 to 3534) Bug#11746602Dmitry Lenev1 Apr