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#11746602 | Dmitry Lenev | 1 Apr |
