Prelocking-free SPs, post-review fixes:

* Don't activate prelocking mode for evaluating procedure arguments when it is not necessary.
* Code structure simplification and cleanup.
* Cleanup in .test files

mysql-test/r/sp-prelocking.result

-drop database if exists testdb;
+drop database if exists mysqltest;
 drop table if exists t1, t2, t3, t4;
 drop procedure if exists sp1;
 drop procedure if exists sp2;
@@ -7,8 +7,8 @@ drop procedure if exists sp4;
 drop function if exists f1;
 drop function if exists f2;
 drop function if exists f3;
-create database testdb;
-use testdb//
+create database mysqltest;
+use mysqltest//
 create procedure sp1 () 
 begin
 drop table if exists t1;
@@ -17,7 +17,7 @@ end;
 //
 select database();
 database()
-testdb
+mysqltest
 call sp1();
 my-col
 1
@@ -25,12 +25,12 @@ Warnings:
 Note	1051	Unknown table 't1'
 select database();
 database()
-testdb
+mysqltest
 use test;
 select database();
 database()
 test
-call testdb.sp1();
+call mysqltest.sp1();
 my-col
 1
 Warnings:
@@ -38,8 +38,8 @@ Note	1051	Unknown table 't1'
 select database();
 database()
 test
-drop procedure testdb.sp1;
-drop database testdb;
+drop procedure mysqltest.sp1;
+drop database mysqltest;
 create procedure sp1() 
 begin 
 create table t1 (a int); 
@@ -95,13 +95,15 @@ create temporary table t1 (a int);
 insert into t1 values(1);
 call sp1();
 select 't1', a from t1;
-select 't2', b from t2;
+select 't2', a from t2;
 drop table t1;
 drop table t2;
 end//
 call sp2();
 t1	a
 t1	1
+t2	a
+t2	1
 drop procedure sp1;
 drop procedure sp2;
 create table t1 (a int);
@@ -138,21 +140,15 @@ end //
 call sp4();
 a
 1
-1
-1
 2
 a
 1
-1
 2
 a
 1
-1
 2
 a
 5
-drop temporary table t1;
-drop temporary table t2;
 drop procedure sp1;
 drop procedure sp2;
 drop procedure sp3;

mysql-test/r/sp-security.result

 use test;
 grant usage on *.* to user1@localhost;
 flush privileges;
-drop table if exists t1,t2;
+drop table if exists t1;
 drop database if exists db1_secret;
 create database db1_secret;
 create procedure db1_secret.dummy() begin end;

mysql-test/r/sp.result

@@ -84,7 +84,6 @@ foo	1
 kaka	3
 delete from t1|
 drop procedure setcontext|
-drop table if exists t3|
 create table t3 ( d date, i int, f double, s varchar(32) )|
 drop procedure if exists nullset|
 create procedure nullset()
@@ -520,7 +519,6 @@ select data into x from test.t1 limit 1;
 insert into test.t3 values ("into4", x);
 end|
 delete from t1|
-drop table if exists t3|
 create table t3 ( s char(16), d int)|
 call into_test4()|
 Warnings:
@@ -564,13 +562,12 @@ insert into test.t1 values (x, y);
 create temporary table test.t3 select * from test.t1;
 insert into test.t3 values (concat(x, "2"), y+2);
 end|
-drop table if exists t3|
 call create_select("cs", 90)|
 select * from t1, t3|
 id	data	id	data
 cs	90	cs	90
 cs	90	cs2	92
-drop table if exists t3|
+drop table t3|
 delete from t1|
 drop procedure create_select|
 drop function if exists e|
@@ -701,7 +698,6 @@ id	data
 hndlr3	13
 delete from t1|
 drop procedure hndlr3|
-drop table if exists t3|
 create table t3 ( id   char(16), data int )|
 drop procedure if exists hndlr4|
 create procedure hndlr4()
@@ -744,7 +740,6 @@ foo	40
 bar	15
 zap	663
 drop procedure cur1|
-drop table if exists t3|
 create table t3 ( s char(16), i int )|
 drop procedure if exists cur2|
 create procedure cur2()
@@ -1308,7 +1303,6 @@ select t1max()|
 t1max()
 5
 drop function t1max|
-drop table if exists t3|
 create table t3 (
 v char(16) not null primary key,
 c int unsigned not null
@@ -1429,7 +1423,6 @@ select @1, @2|
 2	NULL
 drop table t70|
 drop procedure bug1656|
-drop table if exists t3|
 create table t3(a int)|
 drop procedure if exists bug1862|
 create procedure bug1862()
@@ -1554,7 +1547,6 @@ select @x|
 42
 drop procedure bug2776_1|
 drop procedure bug2776_2|
-drop table if exists t3|
 create table t3 (s1 smallint)|
 insert into t3 values (123456789012)|
 Warnings:
@@ -1615,7 +1607,6 @@ f1	rc	t3
 drop procedure bug1863|
 drop temporary table temp_t1;
 drop table t3, t4|
-drop table if exists t3, t4|
 create table t3 ( 
 OrderID  int not null,
 MarketID int,
@@ -1693,7 +1684,6 @@ select @i, from_unixtime(@stamped_time, '%d-%m-%Y %h:%i:%s') as time|
 @i	time
 2	01-01-1970 03:16:40
 drop procedure bug3426|
-drop table if exists t3, t4|
 create table t3 (
 a int primary key, 
 ach char(1)
@@ -1723,7 +1713,6 @@ a	ach	b	bch
 1	a	1	b
 drop procedure bug3448|
 drop table t3, t4|
-drop table if exists t3|
 create table t3 (
 id int unsigned auto_increment not null primary key,
 title VARCHAR(200),
@@ -1872,7 +1861,6 @@ select 1+2|
 1+2
 3
 drop procedure bug3843|
-drop table if exists t3|
 create table t3 ( s1 char(10) )|
 insert into t3 values ('a'), ('b')|
 drop procedure if exists bug3368|
@@ -1888,7 +1876,6 @@ group_concat(v)
 yz,yz
 drop procedure bug3368|
 drop table t3|
-drop table if exists t3|
 create table t3 (f1 int, f2 int)|
 insert into t3 values (1,1)|
 drop procedure if exists bug4579_1|
@@ -1913,7 +1900,6 @@ Warning	1329	No data to FETCH
 drop procedure bug4579_1|
 drop procedure bug4579_2|
 drop table t3|
-drop table if exists t3|
 drop procedure if exists bug2773|
 create function bug2773() returns int return null|
 create table t3 as select bug2773()|
@@ -1935,7 +1921,6 @@ select bug3788()|
 bug3788()
 5
 drop function bug3788|
-drop table if exists t3|
 create table t3 (f1 int, f2 int, f3 int)|
 insert into t3 values (1,1,1)|
 drop procedure if exists bug4726|
@@ -2096,7 +2081,6 @@ call bug4902_2()|
 Id	User	Host	db	Command	Time	State	Info
 #	root	localhost	test	Query	#	NULL	show processlist
 drop procedure bug4902_2|
-drop table if exists t3|
 drop procedure if exists bug4904|
 create procedure bug4904()
 begin
@@ -2285,7 +2269,6 @@ flush status|
 flush query cache|
 delete from t1|
 drop procedure bug3583|
-drop table if exists t3|
 drop procedure if exists bug4905|
 create table t3 (s1 int,primary key (s1))|
 drop procedure if exists bug4905|
@@ -2343,7 +2326,6 @@ call bug8540()|
 y	z
 1	1
 drop procedure bug8540|
-drop table if exists t3|
 create table t3 (s1 int)|
 drop procedure if exists bug6642|
 create procedure bug6642()
@@ -2426,7 +2408,6 @@ call bug7992_2()|
 drop procedure bug7992_1|
 drop procedure bug7992_2|
 drop table t3|
-drop table if exists t3|
 create table t3 (  userid bigint(20) not null default 0 )|
 drop procedure if exists bug8116|
 create procedure bug8116(in _userid int)
@@ -2587,7 +2568,6 @@ delete from t1|
 drop procedure if exists bug6900|
 drop procedure if exists bug9074|
 drop procedure if exists bug6900_9074|
-drop table if exists t3|
 create table t3 (w char unique, x char)|
 insert into t3 values ('a', 'b')|
 create procedure bug6900()

mysql-test/t/sp-prelocking.test

+#
+# Tests of prelocking-free execution of stored procedures.
+# Currently two properties of prelocking-free SP execution are checked:
+#  - It is possible to execute DDL statements in prelocking-free stored
+#    procedure
+#  - The same procedure can be called in prelocking-free mode and 
+#    in prelocked mode (from within a function).
+
 --disable_warnings
-drop database if exists testdb;
+drop database if exists mysqltest;
 drop table if exists t1, t2, t3, t4;
 drop procedure if exists sp1;
 drop procedure if exists sp2;
@@ -12,9 +20,9 @@ drop function if exists f3;
 
 # BUG#8072 
 
-create database testdb;
+create database mysqltest;
 delimiter //;
-use testdb//
+use mysqltest//
 create procedure sp1 () 
 begin
   drop table if exists t1;
@@ -29,11 +37,11 @@ select database();
 
 use test;
 select database();
-call testdb.sp1();
+call mysqltest.sp1();
 select database();
 
-drop procedure testdb.sp1;
-drop database testdb;
+drop procedure mysqltest.sp1;
+drop database mysqltest;
 
 # BUG#8766
 
@@ -96,7 +104,7 @@ begin
   insert into t1 values(1);
   call sp1();
   select 't1', a from t1;
-  select 't2', b from t2;
+  select 't2', a from t2;
   drop table t1;
   drop table t2;
 end//
@@ -151,8 +159,6 @@ end //
 delimiter ;//
 call sp4();
 
-drop temporary table t1;
-drop temporary table t2;
 drop procedure sp1;
 drop procedure sp2;
 drop procedure sp3;

mysql-test/t/sp-security.test

@@ -15,7 +15,7 @@ grant usage on *.* to user1@localhost;
 flush privileges;
 
 --disable_warnings
-drop table if exists t1,t2;
+drop table if exists t1;
 drop database if exists db1_secret;
 --enable_warnings
 # Create our secret database

mysql-test/t/sp.test

@@ -147,9 +147,6 @@ drop procedure setcontext|
 
 
 # Set things to null
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 ( d date, i int, f double, s varchar(32) )|
 
 --disable_warnings
@@ -683,9 +680,6 @@ begin
 end|
 
 delete from t1|
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 ( s char(16), d int)|
 call into_test4()|
 select * from t3|
@@ -741,14 +735,9 @@ begin
   insert into test.t3 values (concat(x, "2"), y+2);
 end|
 
---disable_warnings
-drop table if exists t3|
---enable_warnings
 call create_select("cs", 90)|
 select * from t1, t3|
---disable_warnings
-drop table if exists t3|
---enable_warnings
+drop table t3|
 delete from t1|
 drop procedure create_select|
 
@@ -922,9 +911,6 @@ drop procedure hndlr3|
 # Variables might be uninitialized when using handlers
 # (Otherwise the compiler can detect if a variable is not set, but
 #  not in this case.)
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 ( id   char(16), data int )|
 
 --disable_warnings
@@ -977,9 +963,6 @@ call cur1()|
 select * from t1|
 drop procedure cur1|
 
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 ( s char(16), i int )|
 
 --disable_warnings
@@ -1611,9 +1594,6 @@ insert into t1 values ("foo", 3), ("bar", 2), ("zip", 5), ("zap", 1)|
 select t1max()|
 drop function t1max|
 
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 (
   v char(16) not null primary key,
   c int unsigned not null
@@ -1747,9 +1727,6 @@ drop procedure bug1656|
 #
 # BUG#1862
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3(a int)|
 
 --disable_warnings
@@ -2006,9 +1983,6 @@ drop procedure bug2776_2|
 #
 # BUG#2780
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 (s1 smallint)|
 
 insert into t3 values (123456789012)|
@@ -2082,9 +2056,6 @@ drop table t3, t4|
 #
 # BUG#2656
 #
---disable_warnings
-drop table if exists t3, t4|
---enable_warnings
 
 create table t3 ( 
   OrderID  int not null,
@@ -2172,8 +2143,6 @@ drop procedure bug3426|
 # BUG#3448
 #
 --disable_warnings
-drop table if exists t3, t4|
-
 create table t3 (
   a int primary key, 
   ach char(1)
@@ -2205,9 +2174,6 @@ drop table t3, t4|
 #
 # BUG#3734
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 (
   id int unsigned auto_increment not null primary key,
   title VARCHAR(200),
@@ -2383,9 +2349,6 @@ drop procedure bug3843|
 #
 # BUG#3368
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 ( s1 char(10) )|
 insert into t3 values ('a'), ('b')|
 
@@ -2405,9 +2368,6 @@ drop table t3|
 #
 # BUG#4579
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 (f1 int, f2 int)|
 insert into t3 values (1,1)|
 
@@ -2442,7 +2402,6 @@ drop table t3|
 # BUG#2773: Function's data type ignored in stored procedures
 #
 --disable_warnings
-drop table if exists t3|
 drop procedure if exists bug2773|
 --enable_warnings
 
@@ -2471,10 +2430,6 @@ drop function bug3788|
 #
 # BUG#4726
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
- 
 create table t3 (f1 int, f2 int, f3 int)|
 insert into t3 values (1,1,1)|
 
@@ -2505,9 +2460,6 @@ drop table t3|
 # BUG#4318
 #
 #QQ Don't know if HANDLER commands can work with SPs, or at all...
-#--disable_warnings
-#drop table if exists t3|
-#--enable_warnings
 # 
 #create table t3 (s1 int)|
 #insert into t3 values (3), (4)|
@@ -2588,10 +2540,6 @@ drop procedure bug4902_2|
 #
 # BUG#4904
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
-
 --disable_warnings
 drop procedure if exists bug4904|
 --enable_warnings
@@ -2845,7 +2793,6 @@ drop procedure bug3583|
 # BUG#4905: Stored procedure doesn't clear for "Rows affected"
 #
 --disable_warnings
-drop table if exists t3|
 drop procedure if exists bug4905|
 --enable_warnings
 
@@ -2945,9 +2892,6 @@ drop procedure bug8540|
 #
 # BUG#6642: Stored procedure crash if expression with set function
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 (s1 int)|
 
 --disable_warnings
@@ -3035,9 +2979,6 @@ drop table t3|
 # BUG#8116: calling simple stored procedure twice in a row results
 #           in server crash
 #
---disable_warnings
-drop table if exists t3|
---enable_warnings
 create table t3 (  userid bigint(20) not null default 0 )|
 
 --disable_warnings
@@ -3280,7 +3221,6 @@ delete from t1|
 drop procedure if exists bug6900|
 drop procedure if exists bug9074|
 drop procedure if exists bug6900_9074|
-drop table if exists t3|
 --enable_warnings
 
 create table t3 (w char unique, x char)|

sql/sp.cc

@@ -1176,40 +1176,39 @@ extern "C" byte* sp_sroutine_key(const byte *ptr, uint *plen, my_bool first)
 
 
 /*
-  Check if routines in routines_list require sp_cache_routines_and_add_tables
-  call.
+  Check if
+   - current statement (the one in thd->lex) needs table prelocking
+   - first routine in thd->lex->sroutines_list needs to execute its body in
+     prelocked mode.
 
   SYNOPSIS
-    sp_need_cache_routines()
-      thd
-      routines 
-      need_skip_first  OUT TRUE - don't do prelocking for the 1st element in 
-                                  routines list.
-                           FALSE- otherwise
+    sp_get_prelocking_info()
+      thd                  Current thread, thd->lex is the statement to be
+                           checked.
+      need_prelocking      OUT TRUE  - prelocked mode should be activated
+                                       before executing the statement
+                               FALSE - Don't activate prelocking 
+      first_no_prelocking  OUT TRUE  - Tables used by first routine in
+                                       thd->lex->sroutines_list should be
+                                       prelocked.
+                               FALSE - Otherwise.
   NOTES 
     This function assumes that for any "CALL proc(...)" statement routines_list 
     will have 'proc' as first element (it may have several, consider e.g.
     "proc(sp_func(...)))". This property is currently guaranted by the parser.
-
-  RETURN
-    TRUE  Need to sp_cache_routines_and_add_tables call for this statement.
-    FALSE Otherwise.
 */
 
-bool sp_need_cache_routines(THD *thd, SQL_LIST *routines_list, bool *need_skip_first)
+void sp_get_prelocking_info(THD *thd, bool *need_prelocking, 
+                            bool *first_no_prelocking)
 {
   Sroutine_hash_entry *routine;
-  routine= (Sroutine_hash_entry*)routines_list->first;
-
-  *need_skip_first= FALSE;
-  if (!routine)
-    return FALSE;
+  routine= (Sroutine_hash_entry*)thd->lex->sroutines_list.first;
 
-  if (routine->key.str[0] != TYPE_ENUM_PROCEDURE)
-    return TRUE;
+  DBUG_ASSERT(routine);
+  bool first_is_procedure= (routine->key.str[0] == TYPE_ENUM_PROCEDURE);
 
-  *need_skip_first= TRUE;
-  return TRUE;
+  *first_no_prelocking= first_is_procedure;
+  *need_prelocking= !first_is_procedure || test(routine->next);
 }
 
 

sql/sp.h

@@ -79,8 +79,8 @@ sp_show_status_function(THD *thd, const char *wild);
   Procedures for pre-caching of stored routines and building table list
   for prelocking.
 */
-bool sp_need_cache_routines(THD *thd, SQL_LIST *routines_list, 
-                            bool *need_skip_first);
+void sp_get_prelocking_info(THD *thd, bool *need_prelocking, 
+                            bool *first_no_prelocking);
 void sp_add_used_routine(LEX *lex, Query_arena *arena,
                          sp_name *rt, char rt_type);
 void sp_update_sp_used_routines(HASH *dst, HASH *src);

sql/sp_cache.h

@@ -25,7 +25,8 @@
 /*
   Stored procedures/functions cache. This is used as follows:
    * Each thread has its own cache.
-   * When SP is used it is always in some thread's cache.
+   * Each sp_head object is put into its thread cache after creation and is
+     removed from there on its deletion.
 */
 
 class sp_head;

sql/sp_head.cc

@@ -921,7 +921,8 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 
     /* 
       Okay, got values for all arguments. Close tables that might be used by 
-      arguments evaluation.
+      arguments evaluation. If arguments evaluation required prelocking mode, 
+      we'll leave it here.
     */
     if (!thd->in_sub_stmt)
       close_thread_tables(thd, 0, 0, 0);
@@ -1492,8 +1493,6 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
            instruction if it is not really used.
   */
 
-  bool collect_prelocking_tail= FALSE;
-
   if (thd->prelocked_mode == NON_PRELOCKED)
   {
     /*
@@ -1511,14 +1510,6 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
       *lex_query_tables_own_last= prelocking_tables;
       m_lex->mark_as_requiring_prelocking(lex_query_tables_own_last);
     }
-    else
-    {
-      /* 
-        Let open_tables_calculate list of tables that this statement needs
-        to have prelocked.
-      */
-      collect_prelocking_tail= TRUE;
-    }
   }
     
   reinit_stmt_before_use(thd, m_lex);
@@ -1539,35 +1530,26 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp,
   thd->proc_info="closing tables";
   close_thread_tables(thd);
 
-  if (thd->prelocked_mode == NON_PRELOCKED)
-  {
-    if (!lex_query_tables_own_last)
-      lex_query_tables_own_last= thd->lex->query_tables_own_last;
-      
-    if (lex_query_tables_own_last)
-    {
-      if (collect_prelocking_tail)
+  if (m_lex->query_tables_own_last)
   {
     /*
-          This is the first time this statement has entered/left prelocked
-          mode on its own. open_tables() has calculated the set of tables this
-          statement needs to have prelocked and added them to the end of
-          m_lex->query_tables(->next_global)*.
-          Save this "tail" for subsequent calls (and restore original list 
-          below)
+      We've entered and left prelocking mode when executing statement
+      stored in m_lex. 
+      m_lex->query_tables(->next_global)* list now has a 'tail' - a list
+      of tables that are added for prelocking. (If this is the first
+      execution, the 'tail' was added by open_tables(), otherwise we've
+      attached it above in this function).
+      Now we'll save the 'tail', and detach it.
     */
+    DBUG_ASSERT(!lex_query_tables_own_last ||
+                lex_query_tables_own_last == m_lex->query_tables_own_last &&
+                prelocking_tables == *(m_lex->query_tables_own_last));
+
     lex_query_tables_own_last= m_lex->query_tables_own_last;
     prelocking_tables= *lex_query_tables_own_last;
-      }
-      /*
-        The table list now has list of tables that need to be prelocked
-        when this statement executes, chop it off, and mark this statement 
-        as not requiring prelocking.
-      */
     *lex_query_tables_own_last= NULL;
     m_lex->mark_as_requiring_prelocking(NULL);
   }
-  }
   thd->rollback_item_tree_changes();
 
   /*

sql/sql_base.cc

@@ -1865,25 +1865,23 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter)
           document new prelocked behavior.
   */
   
-  if (!thd->prelocked_mode && !thd->lex->requires_prelocking())
-  {
-    bool first_no_prelocking;
-    if (sp_need_cache_routines(thd, &thd->lex->sroutines_list,
-                               &first_no_prelocking))
+  if (!thd->prelocked_mode && !thd->lex->requires_prelocking() && 
+      thd->lex->sroutines_list.elements)
   {
+    bool first_no_prelocking, need_prelocking;
     TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
 
     DBUG_ASSERT(thd->lex->query_tables == *start);
+    sp_get_prelocking_info(thd, &need_prelocking, &first_no_prelocking);
 
-      if (sp_cache_routines_and_add_tables(thd, thd->lex,
+    if ((sp_cache_routines_and_add_tables(thd, thd->lex,
                                          first_no_prelocking) ||
-          *start)
+        *start) && need_prelocking)
     {
       query_tables_last_own= save_query_tables_last;
       *start= thd->lex->query_tables;
     }
   }
-  }
 
   for (tables= *start; tables ;tables= tables->next_global)
   {
@@ -1917,8 +1915,9 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter)
           2) Tables used by all stored routines that this statement invokes on
              execution.
           We need to know where the bound between these two parts is. If we've
-          just opened the last table in part #1, and it added tables after
-          itself, adjust the boundary pointer accordingly.
+          just opened a view, which was the last table in part #1, and it
+          has added its base tables after itself, adjust the boundary pointer
+          accordingly.
         */
         if (query_tables_last_own &&
             query_tables_last_own == &(tables->next_global) &&

sql/sql_class.cc

@@ -174,11 +174,11 @@ THD::THD()
   :Statement(CONVENTIONAL_EXECUTION, 0, ALLOC_ROOT_MIN_BLOCK_SIZE, 0),
    Open_tables_state(),
    lock_id(&main_lock_id),
-   user_time(0), global_read_lock(0), is_fatal_error(0),
+   user_time(0), in_sub_stmt(FALSE), global_read_lock(0), is_fatal_error(0),
    rand_used(0), time_zone_used(0),
    last_insert_id_used(0), insert_id_used(0), clear_next_insert_id(0),
    in_lock_tables(0), bootstrap(0), derived_tables_processing(FALSE),
-   spcont(NULL), in_sub_stmt(FALSE)
+   spcont(NULL)
 {
   current_arena= this;
   host= user= priv_user= db= ip= 0;