From: Dmitry Lenev Date: April 1 2011 1:47pm Subject: bzr commit into mysql-trunk branch (Dmitry.Lenev:3534) Bug#11746602 List-Archive: http://lists.mysql.com/commits/134469 X-Bug: 11746602 Message-Id: <20110401134742.F05477405D1@bandersnatch> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1442133461==" --===============1442133461== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #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 consequtive 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 13:47:34 +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 13:47:34 +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 13:47:34 +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 13:47:34 +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 13:47:34 +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)) --===============1442133461== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/dmitry.lenev@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: dmitry.lenev@stripped # target_branch: file:///home/dlenev/src/bzr/mysql-trunk-bug27480-3/ # testament_sha1: c098decf28399bb7c69ca93eee130f925f77a39b # timestamp: 2011-04-01 17:47:42 +0400 # base_revision_id: dmitry.lenev@stripped\ # mn6r9cxwywygq3f8 # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWZ3rJ+EAB3pfgFAQeff//3/n 3+C////wYBCE+8qMVdzKXA6AAO4hQWxdnOwqgtjX0ddsGrMpINUtCUhTNTypsU9Ceo2mTTUNGgGg NAANGgNAkiTQYgBBNIxpGpqeU02p+qMNRvVAyeoGRocZMmhiMTRgEYCYQBgJpo0yNAMIkiKaU/RP TKek9TaU8Jqe0jT1GSHkjQaNDQyBoMqRoBoBoGmT0gBoDQ0BoAAABUkgECmaaNNTBAam0NKmynpP KfqjPVGNR6mj1HqMEOxQGpRCovzXpyogttWtgKzGbrd+96qrVAVkOGIqKR5GHVDJlXMfWj0YZ1bU a5a0YUqy6aj7z6LJga9j0YPa6TEpSjxzFOnP4PHj0ndkZDnzJuKMXEMMAwwc+cZoOrAaMBDfxvXj nmd2s4Seq7a2dsrfzIVdh2M7XtXoduFfrw4tbr869OhC8H/eSFZv5pd/R3GjhDQRoNgZzsT4JxJC MZJF4fih5r7rOe+vkufPV+SYXxrE55uQwgbnNUtWMqSZCXw/7gOZM+x/Gmjhi73RlnM8z9HwfLCq n2fnOLofBZk+xsc78CGc6zz+QosTcHOe4h1+8u6Qh8AsQLEIWqP2w6vaYGSlKlKZtXjGLrYLnBxX vg7tbozim9i7Xn9HJSn3sGn4WvNeZbNZZslfe6FGDJ03v4NuRsN2OLRTa1MWUdlWZdNyYSl6zl+7 WAiRQdYsDGY7z/IkFoVmuAXi80pGJsir4oqLoBfXU5DqmZhTKGRE8ig5Wq8KIkVejBgj1FSsDzUo 32FZIIPDUqMysIOOUFErJdjy1PBTg16udq9LNdhiw70vUkU7lk7n6tz8XSULCwPaVmId6obiqXBd CUTlpWZnIR3h65JV9GmanY5fz9mkVCuB1OpZn6KaXdM4KwKvswXKBZFELkhL5KKHWh4LRyhZdHGE GmAkG4CLJx8s+nwaOzs06Dve2Q9W99yTbPXk5VMIifY6AhJJJJJJJJJCEkk1+SuPUUcYweA0lXHD sLC/MKOyPs/PB9VxxcHsTXfbbVZH+i9/JsN3yZLdTyan3OP46LEd6diTJqSrpSR4Tu4y07ptuiyH Nt7lD72aAN7llbN56TqDfOvFfMOq7Q5ni4szt8nN6vHYsZK1sn8VMHRbfpeydnNcxqFUwUkZLyOh zR5VHi9b1RplpGrU7TSqW2N2kHRUStHVPLG8v58RtxjHt2dA2y0mQbDQJYWjKXXHdY5CMhhCtzKx dqpSswiqqrIFc04oFw8wn7CTigdo7wWKV6iYjkCZdBzJBTMTkmcR5DAWsZ0xVs0jOqcBUBDmpivZ EQFYfHKNfvCCVZi0IvwjRz8LxJx9kmRfUHnTIWJkizy4kZmHlZWacoXXQV9xE5kftv9ULNl7hM9G 6At0MVjmZEiZjGyIEt4AuN5ywEpiVH1vxfhbkVwEySJstvjSUTVfMJuQU3iAcWJ9YuQ1DGtuI29H EYt7GXXNLgn2vtwagHEVpj+epM0Zr13q8irFpN7Qw7JDPeW4W8+WQrqzlpDAvrJMEojQHHwEmEnj ZdwVVWSSiaZZCtJ3jKZE7xj/odFxXBTWXRaRXJaSzMXEsd8q9xJ0uRGzlHl1mVVGBvJFHZ7qQ5hW WCam08RKoPWW3TCbFnVGwqi82IrwWRWxDmsl9CZUYucKC6jGqtNauBxKv/LPht8F0VChQVKdlOVl hTezIy3CzUbUTcyCV0HH91CHBROZ3Eiz+asrz5cqYVtgQNWZWjK0sSmZiuOWBYYmeHAsvfh6sLqY 0JWw8IvIyowzx52WMKcbTwWRDSQ/US5BisVspT4O782cg4sBjpstkMJosS4wCZ5LZJb2mpXeDGBM MExQdEWllK+pKRqw44UKBlUdP0X1LwXtX6loc5SekN9XdOJjCK3MxiN3cHIs5W6z3vIHRVNcrBLG eMiy5KYq8ak1wVWIkWwF65ORtE4S9sp0MGbpCh6gtJSXE2dbYOvMMCZWV4XWRkk2qL7Wm1l0RQSq hbUlBDxZxRYtdmrQWeAXPlcfF/yXLlKfgpZm3LFlnyYKWUwXvfHue3UcvQOY5T34BjB8Du4FQkun NREJIe0PtNvrTYaYpmPQicvp51E3QMA1EP9PiPuB/JWhxUNRieOOikiwo+KiYV8h/UblkwJA3imj EKtFBn/f1tj0SR+y5ZUXLFFFFKUtIdbN+yYSHowP3J/B+hpJOCaM0Lhrf8sUwT92qLnMS9cip+6m 9Q1NF7mcWDBg4LiYRM0cy5ZzEmoqScUbnUmAwTHu8YH7m6rUJCLCDwFGRGhhAMkyODROYYQvUWRk n/jqklOhex9OxGRKTjJOpgpSlFy9FQbSZyHOmsmDc2DkjRemckqRgp/Ze6k2W5icoxbXOl1mQsYy D/hpaNcJwJaScBksyWS04P/mC9fE4NZLLNqTbEUjmT4o8nzRtTnTbSUgYUMSX99KYex/gCh3Dv1E zHs9c+0mC/efMS9DH+JX/af8C2fn+hO/5r8P8WcRC3ycAsI8Yjew6jk/zZLuowjizWShm1M3+85l o2v6R9X9dbzr0+in3xnGy5uxHwUOdgvORfq6ZDnSpOiYkwXylKc0azn1lWz9BsgtF8+znNevqkUJ otXRfZ1PCsmYkNjM+9fjgePebjjjKH4jvZClBhGNJhYE6QHi5JARDGlSW0slaYo0yWB/fPWhnN1Q 8je6a/RZwc7qWfdTNc3bvFUnwbZ/qTWpyV75g4s1GFla4ynjfVL1JhWqYIM0C3iWriacESzY3lTp M1k2PyMTLAyME1hmueUl4BAhApiUUQpMCiloovy1FvBDiwc3j38TUUkuZ2KQYX0S7DEtcuQuZ9hS nrRi5N+1718psck69256sWxSPPsT+r+yVFtHvSfq3HDe487t9tmTveh5LH3ZL/Ne7o07j4ta9OvI xz0Y0rGaUYscFX+T4N01y9m0opkLVJmspFSlRGyo9z2ZuZc8lz1aTDmythZl2lx9KM0b3how8REC wQ44xnRHkTvMTdd6GUfrPa0LIrWUVs6rLV1L9bd6DBTofBcXFXt9+QVFTZ6N06K8wOeIMbqfDa/R m979QJJXB0Q1qUUpUh1uuMqTbxYT88iK7Iak8JYLSffKGWTtVOHfYzbJx89bS+Zb0zLsWrcMpSUp 7d83SNads1C1S6MocOSRcK0jgm5Sl0ZSM/ajwifN7ntt6ZjPcvcnmmD2KbXhranjGieNOtNbsn6R 5ecVGyPJX9/H0ToOsiHoDIDVi7RxLQg9fUZCZm9y9/RVUVpJMHHmoSGB0mGo9SebW+kOjIHM9Eeq iQkYpEnUH1XGx+EqWn5KHLBlPmvm74y6Hyvkz7JdMkNgzyk9qTR+JqnzrdXfOVmZshd0cempJotl 8ExdzPtemI5zYzeqTfTEqeoUu3NuWEnyU95tmYEjwRv7u3DIRyZkmFoYZbhsi9F4NCsHhOIYHRGD hJGzVNnA+l8acnhaYJMfW75h9FkojCSYCZXMMMkMJIM4pEqlRAAfACXfYZK5Twgma1uLetHVL5Kn Sip1HGbZqGmOE8swzRVR6JGh9ac22OMhVRhEWiykU75Y1DzIvC986Zc60jpUq3PNHtunVTbU+MbZ OCggdD3mKYp4TDTxcby2LtFGXtd9LESECGxZIqxbohjPe9UvbY54Tk4p0uUxfZ3kb9V9KjeDrGiv oJwVDFdVZmIVauPoO2hcMMMQhlI5SxUCkhJeycRNrzucMOQwGZFozxrli5r1ULSka0evKTHBIt64 wkL8Vjd0NMkeud0+EwmkfZqbIa1JeuLoulVJFTUWnx5du/rd6Gy06XmtSgxplOMu9kbpUnJo6UYT 2R7JpgkxkjYqbzNSepXlSLj143GMxSZdkZTxUXGoZXVYLn1iN43E34XHMS4EFYFaiuOYLUt8sjkL Q4CgN6DVEDuUNO3ecYNh09qe60VJpFpaL5kyTwcyHNPymcIiW9jqCR0KDpE/lUvk7p7PEOKuXPFc NUhV4LT0nB4VOiAFHihRHNOSeQncLqF7Qs7iJCAa+cKBs8LvWtVawOBIczO4o0BiNz3GKXRKS5So fSO/xuTWdPY2sygvlTdNT5y/DHqSZ3HOmRimlFS0eiKWXVn5y9arqSyoiShuam5rJkmQMSxiBckL tWd/AuRw/RZbJZ8PfjhoqZrJVWrS1UzO+XtyL/Xn9OEZQ8pl+U0Pejm2lFWXN3K7AvhdHjXGNk9N AzASyT69C+fhcWGBydadUy1VVVVVde+97ZvgSpdtVNXOx4EMoEhGMI6UpyrnIyBpcF/heibRcsVH 0XX9V56VUXM1ms1r74wLR4Mi9wzsVFp13XTnptHcpSn5FQtVSlKUG/3YRq4xpF02yrkVnDnKN22y p81hYZsedUGo0pZeVwV9yt+EGkNEzni0xd1Sx2nYuWzJb6cjoVsjBI7FfKQixpuahAYNbiAQ1uS9 D5Dh0BkO07gcRWiUG1RmuF7fKvW37LlRF0jBUbqkh9yb2LusRJcAvJHiMfDgfsPRoLkrVaLpak2d A0Zf1yLK+LpRuufKb/wlRGTpOM3O7b9MDUzHXL55ma+kZSnVPY1myTteCzjROqUWKZmRunUID4L3 h4h5gYoZG2W31KIiO67VWaKJa5qAPa8YX0MIcQUlAQiryvEL4A3x2o4RUJrIWDFwX2kVhqv/i7ki nChITvWT8IA= --===============1442133461==--