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:
  Prelocking-free SPs, post-review fixes:
  Added comment, s/testdb/mysqltest/, fixed a wrong test (error wasnt reported because of known bug in mysqltestrun)
mysql-test/r/sp-security.result:
  Don't drop the table we're not using.
mysql-test/r/sp.result:
  Prelocking-free SPs, post-review fixes:
  remove redundant "drop table if exists t3" statements
mysql-test/t/sp-prelocking.test:
  Prelocking-free SPs, post-review fixes:
  Added comment, s/testdb/mysqltest/, fixed a wrong test (error wasnt reported because of known bug in mysqltestrun)
mysql-test/t/sp-security.test:
  Don't drop the table we're not using.
mysql-test/t/sp.test:
  Prelocking-free SPs, post-review fixes:
  remove redundant "drop table if exists t3" statements
sql/sp.cc:
  New, better defined, sp_get_prelocking_info() function to get info about
    statement prelocking options
sql/sp.h:
  Prelocking-free SPs, post-review fixes: New, better defined, sp_get_prelocking_info() 
  function to get info about statement prelocking options
sql/sp_cache.h:
  Prelocking-free SPs, post-review fixes: Amended the comments
sql/sp_head.cc:
  Prelocking-free SPs, post-review fixes: Amend the comments, simplify the code that 
  attaches removes statement's prelocking tables.
sql/sql_base.cc:
  Prelocking-free SPs, post-review fixes:
  * Use a better defined sp_get_prelocking_info() function to get info about 
    statement prelocking options
  * Don't activate prelocked mode for evaluation of SP arguments that use tables 
    but don't need prelocking.
sql/sql_class.cc:
  Prelocking-free SPs, post-review fixes: Initialize THD members in the order they are declared.

6 files changed, 62 insertions, 81 deletions

diff --git a/sql/sp.cc b/sql/sp.cc
index 3d513b16d07..0cc1c988217 100644
--- a/sql/sp.cc
+++ b/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);
 }
 
 
diff --git a/sql/sp.h b/sql/sp.h
index 716f3d90f55..3c837f8b586 100644
--- a/sql/sp.h
+++ b/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);
diff --git a/sql/sp_cache.h b/sql/sp_cache.h
index 5873c763289..1ea71160a3a 100644
--- a/sql/sp_cache.h
+++ b/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;
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 8d56e2a0b38..7002bc7b020 100644
--- a/sql/sp_head.cc
+++ b/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,34 +1530,25 @@ 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 (m_lex->query_tables_own_last)
   {
-    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)
-      {
-        /*
-          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)
-        */
-        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);
-    }
+    /*
+      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;
+    *lex_query_tables_own_last= NULL;
+    m_lex->mark_as_requiring_prelocking(NULL);
   }
   thd->rollback_item_tree_changes();
 
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 367bd2c5ade..90e31edd9bb 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1865,23 +1865,21 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter)
           document new prelocked behavior.
   */
   
-  if (!thd->prelocked_mode && !thd->lex->requires_prelocking())
+  if (!thd->prelocked_mode && !thd->lex->requires_prelocking() && 
+      thd->lex->sroutines_list.elements)
   {
-    bool first_no_prelocking;
-    if (sp_need_cache_routines(thd, &thd->lex->sroutines_list,
-                               &first_no_prelocking))
-    {
-      TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
+    bool first_no_prelocking, need_prelocking;
+    TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
 
-      DBUG_ASSERT(thd->lex->query_tables == *start);
+    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,
-                                           first_no_prelocking) ||
-          *start)
-      {
-        query_tables_last_own= save_query_tables_last;
-        *start= thd->lex->query_tables;
-      }
+    if ((sp_cache_routines_and_add_tables(thd, thd->lex,
+                                         first_no_prelocking) ||
+        *start) && need_prelocking)
+    {
+      query_tables_last_own= save_query_tables_last;
+      *start= thd->lex->query_tables;
     }
   }
 
@@ -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) &&
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 81cdc6562e9..9b0decb3950 100644
--- a/sql/sql_class.cc
+++ b/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;