Apply and review:

3655 Jon Olav Hauglid   2009-10-19
Bug #30977 Concurrent statement using stored function and DROP FUNCTION 
           breaks SBR
Bug #48246 assert in close_thread_table

Implement a fix for:
Bug #41804 purge stored procedure cache causes mysterious hang for many
           minutes
Bug #49972 Crash in prepared statements

The problem was that concurrent execution of DML statements that
use stored functions and DDL statements that drop/modify the same
function might result in incorrect binary log in statement (and
mixed) mode and therefore break replication.

This patch fixes the problem by introducing metadata locking for
stored procedures and functions. This is similar to what is done
in Bug#25144 for views. Procedures and functions now are
locked using metadata locks until the transaction is either
committed or rolled back. This prevents other statements from
modifying the procedure/function while it is being executed. This
provides commit ordering - guaranteeing serializability across
multiple transactions and thus fixes the reported binlog problem.

Note that we do not take locks for top-level CALLs. This means
that procedures called directly are not protected from changes by
simultaneous DDL operations so they are executed at the state they
had at the time of the CALL. By not taking locks for top-level
CALLs, we still allow transactions to be started inside
procedures.

This patch also changes stored procedure cache invalidation.
Upon a change of cache version, we no longer invalidate the entire
cache, but only those routines which we use, only when a statement
is executed that uses them.

This patch also changes the logic of prepared statement validation.
A stored procedure used by a prepared statement is now validated
only once a metadata lock has been acquired. A version mismatch
causes a flush of the obsolete routine from the cache and
statement reprepare.
Incompatible changes:
1) ER_LOCK_DEADLOCK is reported for a transaction trying to access
   a procedure/function that is locked by a DDL operation in
   another connection.

2) Procedure/function DDL operations are now prohibited in LOCK
   TABLES mode as exclusive locks must be taken all at once and
   LOCK TABLES provides no way to specifiy procedures/functions to
   be locked.

Test cases have been added to sp-lock.test and rpl_sp.test.

Work on this bug has very much been a team effort and this patch
includes and is based on contributions from Davi Arnaut, Dmitry
Lenev, Magne Mæhre and Konstantin Osipov.

mysql-test/r/ps_ddl.result

@@ -269,8 +269,6 @@ Part 7: TABLE -> TABLE (TRIGGER dependencies) transitions
 =====================================================================
 # Test 7-a: dependent PROCEDURE has changed
 #
-# Note, this scenario is not supported, subject of Bug#12093
-#
 create table t1 (a int);
 create trigger t1_ai after insert on t1 for each row
 call p1(new.a);
@@ -282,10 +280,9 @@ drop procedure p1;
 create procedure p1 (a int) begin end;
 set @var= 2;
 execute stmt using @var;
-ERROR 42000: PROCEDURE test.p1 does not exist
 # Cleanup
 drop procedure p1;
-call p_verify_reprepare_count(0);
+call p_verify_reprepare_count(1);
 SUCCESS
 
 # Test 7-b: dependent FUNCTION has changed
@@ -361,11 +358,13 @@ set @var=8;
 # XXX: bug, the SQL statement in the trigger is still
 # pointing at table 't3', since the view was expanded
 # at first statement execution.
+# Since the view definition is inlined in the statement
+# at prepare, changing the view definition does not cause 
+# repreparation.
 # Repreparation of the main statement doesn't cause repreparation
 # of trigger statements.
 execute stmt using @var;
-ERROR 42S02: Table 'test.t3' doesn't exist
-call p_verify_reprepare_count(1);
+call p_verify_reprepare_count(0);
 SUCCESS
 
 #
@@ -382,6 +381,7 @@ select * from t3;
 a
 6
 7
+8
 flush table t1;
 set @var=9;
 execute stmt using @var;
@@ -396,6 +396,7 @@ select * from t3;
 a
 6
 7
+8
 drop view v1;
 drop table t1,t2,t3;
 # Test 7-d: dependent TABLE has changed

mysql-test/r/ps_ddl1.result

@@ -460,7 +460,7 @@ create schema mysqltest;
 end|
 execute stmt;
 ERROR 42000: PROCEDURE test.p1 does not exist
-call p_verify_reprepare_count(1);
+call p_verify_reprepare_count(0);
 SUCCESS
 
 execute stmt;

mysql-test/r/sp-error.result

@@ -512,7 +512,7 @@ select * from t1;
 end|
 lock table t1 read|
 alter procedure bug9566 comment 'Some comment'|
-ERROR HY000: Table 'proc' was not locked with LOCK TABLES
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
 unlock tables|
 drop procedure bug9566|
 drop procedure if exists bug7299|

mysql-test/suite/rpl/r/rpl_sp.result

@@ -977,4 +977,47 @@ drop procedure mysqltestbug36570_p1;
 drop procedure ` mysqltestbug36570_p2`;
 drop function mysqltestbug36570_f1;
 End of 5.0 tests
-End of 5.1 tests
+# End of 5.1 tests
+#
+# Test Bug#30977 Concurrent statement using stored
+# function and DROP FUNCTION breaks SBR.
+#
+# Demonstrate that stored function DDL can not go through,
+# or, worse yet, make its way into the binary log, while
+# the stored function is in use.
+# For that, try to insert a result of a stored function
+# into a table. Block the insert in the beginning, waiting
+# on a table lock. While insert is blocked, attempt to
+# drop the routine. Verify that this attempt 
+# blocks and waits for INSERT to complete. Commit and 
+# reap the chain of events. Master and slave must contain
+# identical data. Statements in the binrary log must be
+# consistent with data in the table.
+#
+# --> connection default
+drop table if exists t1, t2;
+drop function if exists t1;
+create table t1 (a int);
+create table t2 (a int) as select 1 as a;
+create function f1() returns int deterministic return (select max(a) from t2);
+lock table t2 write;
+# --> connection master
+# Sending 'insert into t1 (a) values (f1())'...
+insert into t1 (a) values (f1());
+# Waitng for 'insert into t1 ...' to get blocked on table lock...
+# Sending 'drop function f1'. It will abort the table lock wait.
+drop function f1;
+# --> connection default
+# Now let's let 'insert' go through...
+unlock tables;
+# --> connection con1
+# Reaping 'insert into t1 (a) values (f1())'...
+ERROR 42000: FUNCTION test.f1 does not exist
+select * from t1;
+a
+select * from  t1;
+a
+drop table t1, t2;
+drop function f1;
+ERROR 42000: FUNCTION test.f1 does not exist
+# End of 5.5 tests.

mysql-test/suite/rpl/t/rpl_sp.test

@@ -621,7 +621,64 @@ drop procedure mysqltestbug36570_p1;
 drop procedure ` mysqltestbug36570_p2`;
 drop function mysqltestbug36570_f1;
 --echo End of 5.0 tests
---echo End of 5.1 tests
+--echo # End of 5.1 tests
+--echo #
+--echo # Test Bug#30977 Concurrent statement using stored
+--echo # function and DROP FUNCTION breaks SBR.
+--echo #
+--echo # Demonstrate that stored function DDL can not go through,
+--echo # or, worse yet, make its way into the binary log, while
+--echo # the stored function is in use.
+--echo # For that, try to insert a result of a stored function
+--echo # into a table. Block the insert in the beginning, waiting
+--echo # on a table lock. While insert is blocked, attempt to
+--echo # drop the routine. Verify that this attempt 
+--echo # blocks and waits for INSERT to complete. Commit and 
+--echo # reap the chain of events. Master and slave must contain
+--echo # identical data. Statements in the binrary log must be
+--echo # consistent with data in the table.
+--echo #
+--echo # --> connection default
+connection default;
+--disable_warnings
+drop table if exists t1, t2;
+drop function if exists t1;
+--enable_warnings
+create table t1 (a int);
+create table t2 (a int) as select 1 as a;
+create function f1() returns int deterministic return (select max(a) from t2);
+lock table t2 write;
+--echo # --> connection master
+connection master;
+--echo # Sending 'insert into t1 (a) values (f1())'...
+--send insert into t1 (a) values (f1())
+connection master1;
+--echo # Waitng for 'insert into t1 ...' to get blocked on table lock...
+let $wait_condition=select count(*)=1 from information_schema.processlist
+where state='Table lock' and info='insert into t1 (a) values (f1())';
+--source include/wait_condition.inc
+--echo # Sending 'drop function f1'. It will abort the table lock wait.
+drop function f1;
+--echo # --> connection default
+connection default;
+--echo # Now let's let 'insert' go through...
+unlock tables;
+--echo # --> connection con1
+connection master;
+--echo # Reaping 'insert into t1 (a) values (f1())'...
+--error ER_SP_DOES_NOT_EXIST
+--reap
+connection master1;
+select * from t1;
+connection slave;
+select * from  t1;
+connection master;
+drop table t1, t2;
+--error ER_SP_DOES_NOT_EXIST
+drop function f1;
+
+--echo # End of 5.5 tests.
+
 
 # Cleanup
 sync_slave_with_master;

mysql-test/t/ps_ddl.test

@@ -278,8 +278,6 @@ deallocate prepare stmt;
 
 --echo # Test 7-a: dependent PROCEDURE has changed
 --echo #
---echo # Note, this scenario is not supported, subject of Bug#12093
---echo #
 
 create table t1 (a int);
 create trigger t1_ai after insert on t1 for each row
@@ -291,11 +289,10 @@ execute stmt using @var;
 drop procedure p1;
 create procedure p1 (a int) begin end;
 set @var= 2;
---error ER_SP_DOES_NOT_EXIST
 execute stmt using @var;
 --echo # Cleanup
 drop procedure p1;
-call p_verify_reprepare_count(0);
+call p_verify_reprepare_count(1);
 
 --echo # Test 7-b: dependent FUNCTION has changed
 --echo #
@@ -355,11 +352,13 @@ set @var=8;
 --echo # XXX: bug, the SQL statement in the trigger is still
 --echo # pointing at table 't3', since the view was expanded
 --echo # at first statement execution.
+--echo # Since the view definition is inlined in the statement
+--echo # at prepare, changing the view definition does not cause 
+--echo # repreparation.
 --echo # Repreparation of the main statement doesn't cause repreparation
 --echo # of trigger statements.
---error ER_NO_SUCH_TABLE
 execute stmt using @var;
-call p_verify_reprepare_count(1);
+call p_verify_reprepare_count(0);
 --echo #
 --echo # Sic: the insert went into t3, even though the view now
 --echo # points at t2. This is because neither the merged view

mysql-test/t/ps_ddl1.test

@@ -363,7 +363,7 @@ end|
 delimiter ;|
 --error ER_SP_DOES_NOT_EXIST
 execute stmt;
-call p_verify_reprepare_count(1);
+call p_verify_reprepare_count(0);
 --error ER_SP_DOES_NOT_EXIST
 execute stmt;
 call p_verify_reprepare_count(0);

mysql-test/t/sp-error.test

@@ -723,7 +723,7 @@ lock table t1 read|
 # This should fail since we forgot to lock mysql.proc for writing
 # explicitly, and we can't open mysql.proc for _writing_ if there
 # are locked tables.
---error 1100
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
 alter procedure bug9566 comment 'Some comment'|
 unlock tables|
 # This should succeed

sql/lock.cc

@@ -980,6 +980,57 @@ void unlock_table_names(THD *thd)
 }
 
 
+/**
+  Obtain an exclusive metadata lock on the stored routine name.
+
+  @param thd         Thread handle.
+  @param is_function Stored routine type (only functions or procedures
+                     are name-locked.
+  @param db          The schema the routine belongs to.
+  @param name        Routine name.
+
+  This function assumes that no metadata locks were acquired
+  before calling it. Additionally, it cannot be called while
+  holding LOCK_open mutex. Both these invariants are enforced by
+  asserts in MDL_context::acquire_exclusive_locks().
+  To avoid deadlocks, we do not try to obtain exclusive metadata
+  locks in LOCK TABLES mode, since in this mode there may be
+  other metadata locks already taken by the current connection,
+  and we must not wait for MDL locks while holding locks.
+
+  @retval FALSE  Success.
+  @retval TRUE   Failure: we're in LOCK TABLES mode, or out of memory,
+                 or this connection was killed.
+*/
+
+bool lock_routine_name(THD *thd, bool is_function,
+                       const char *db, const char *name)
+{
+  MDL_key::enum_mdl_namespace mdl_type= (is_function ?
+                                         MDL_key::FUNCTION :
+                                         MDL_key::PROCEDURE);
+  MDL_request mdl_request;
+
+  if (thd->locked_tables_mode)
+  {
+    my_message(ER_LOCK_OR_ACTIVE_TRANSACTION,
+               ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
+    return TRUE;
+  }
+
+  DBUG_ASSERT(name);
+  DEBUG_SYNC(thd, "before_wait_locked_pname");
+
+  mdl_request.init(mdl_type, db, name, MDL_EXCLUSIVE);
+
+  if (thd->mdl_context.acquire_exclusive_lock(&mdl_request))
+    return TRUE;
+
+  DEBUG_SYNC(thd, "after_wait_locked_pname");
+  return FALSE;
+}
+
+
 static void print_lock_error(int error, const char *table)
 {
   int textno;

sql/mysql_priv.h

@@ -2111,6 +2111,9 @@ void broadcast_refresh(void);
 bool lock_table_names(THD *thd, TABLE_LIST *table_list);
 void unlock_table_names(THD *thd);
 
+/* Lock based on stored routine name */
+bool lock_routine_name(THD *thd, bool is_function, const char *db,
+                       const char *name);
 
 /* old unireg functions */
 

sql/sp.cc

@@ -753,6 +753,11 @@ sp_create_routine(THD *thd, int type, sp_head *sp)
   */
   thd->clear_current_stmt_binlog_row_based();
 
+  /* Grab an exclusive MDL lock. */
+  if (lock_routine_name(thd, type == TYPE_ENUM_FUNCTION,
+                        sp->m_db.str, sp->m_name.str))
+    DBUG_RETURN(SP_OPEN_TABLE_FAILED);
+
   saved_count_cuted_fields= thd->count_cuted_fields;
   thd->count_cuted_fields= CHECK_FIELD_WARN;
 
@@ -919,7 +924,10 @@ sp_create_routine(THD *thd, int type, sp_head *sp)
     ret= SP_OK;
     if (table->file->ha_write_row(table->record[0]))
       ret= SP_WRITE_ROW_FAILED;
-    else if (mysql_bin_log.is_open())
+    if (ret == SP_OK)
+      sp_cache_invalidate();
+
+    if (ret == SP_OK && mysql_bin_log.is_open())
     {
       thd->clear_error();
 
@@ -948,7 +956,6 @@ sp_create_routine(THD *thd, int type, sp_head *sp)
                         FALSE, FALSE, 0);
       thd->variables.sql_mode= 0;
     }
-
   }
 
 done:
@@ -994,6 +1001,11 @@ sp_drop_routine(THD *thd, int type, sp_name *name)
   */
   thd->clear_current_stmt_binlog_row_based();
 
+  /* Grab an exclusive MDL lock. */
+  if (lock_routine_name(thd, type == TYPE_ENUM_FUNCTION,
+                        name->m_db.str, name->m_name.str))
+    DBUG_RETURN(SP_DELETE_ROW_FAILED);
+
   if (!(table= open_proc_table_for_update(thd)))
     DBUG_RETURN(SP_OPEN_TABLE_FAILED);
   if ((ret= db_find_routine_aux(thd, type, name, table)) == SP_OK)
@@ -1006,6 +1018,20 @@ sp_drop_routine(THD *thd, int type, sp_name *name)
   {
     write_bin_log(thd, TRUE, thd->query(), thd->query_length());
     sp_cache_invalidate();
+
+    /*
+      A lame workaround for lack of cache flush:
+      make sure the routine is at least gone from the
+      local cache.
+    */
+    {
+      sp_head *sp;
+      sp_cache **spc= (type  == TYPE_ENUM_FUNCTION ?
+                       &thd->sp_func_cache : &thd->sp_proc_cache);
+      sp= sp_cache_lookup(spc, name);
+      if (sp)
+        sp_cache_flush_obsolete(spc, &sp);
+    }
   }
 
   close_thread_tables(thd);
@@ -1041,6 +1067,12 @@ sp_update_routine(THD *thd, int type, sp_name *name, st_sp_chistics *chistics)
 
   DBUG_ASSERT(type == TYPE_ENUM_PROCEDURE ||
               type == TYPE_ENUM_FUNCTION);
+
+  /* Grab an exclusive MDL lock. */
+  if (lock_routine_name(thd, type == TYPE_ENUM_FUNCTION,
+                        name->m_db.str, name->m_name.str))
+    DBUG_RETURN(SP_OPEN_TABLE_FAILED);
+
   /*
     This statement will be replicated as a statement, even when using
     row-based replication. The flag will be reset at the end of the
@@ -1052,6 +1084,30 @@ sp_update_routine(THD *thd, int type, sp_name *name, st_sp_chistics *chistics)
     DBUG_RETURN(SP_OPEN_TABLE_FAILED);
   if ((ret= db_find_routine_aux(thd, type, name, table)) == SP_OK)
   {
+    if (type == TYPE_ENUM_FUNCTION && ! trust_function_creators &&
+        mysql_bin_log.is_open() &&
+        (chistics->daccess == SP_CONTAINS_SQL ||
+         chistics->daccess == SP_MODIFIES_SQL_DATA))
+    {
+      char *ptr;
+      bool is_deterministic;
+      ptr= get_field(thd->mem_root,
+                     table->field[MYSQL_PROC_FIELD_DETERMINISTIC]);
+      if (ptr == NULL)
+      {
+        ret= SP_INTERNAL_ERROR;
+        goto err;
+      }
+      is_deterministic= ptr[0] == 'N' ? FALSE : TRUE;
+      if (!is_deterministic)
+      {
+        my_message(ER_BINLOG_UNSAFE_ROUTINE,
+                   ER(ER_BINLOG_UNSAFE_ROUTINE), MYF(0));
+        ret= SP_INTERNAL_ERROR;
+        goto err;
+      }
+    }
+
     store_record(table,record[1]);
     table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET;
     ((Field_timestamp *)table->field[MYSQL_PROC_FIELD_MODIFIED])->set_time();
@@ -1077,7 +1133,7 @@ sp_update_routine(THD *thd, int type, sp_name *name, st_sp_chistics *chistics)
     write_bin_log(thd, TRUE, thd->query(), thd->query_length());
     sp_cache_invalidate();
   }
-
+err:
   close_thread_tables(thd);
   DBUG_RETURN(ret);
 }
@@ -1161,10 +1217,7 @@ sp_drop_db_routines(THD *thd, char *db)
 bool
 sp_show_create_routine(THD *thd, int type, sp_name *name)
 {
-  bool err_status= TRUE;
   sp_head *sp;
-  sp_cache **cache = type == TYPE_ENUM_PROCEDURE ?
-                     &thd->sp_proc_cache : &thd->sp_func_cache;
 
   DBUG_ENTER("sp_show_create_routine");
   DBUG_PRINT("enter", ("name: %.*s",
@@ -1174,28 +1227,29 @@ sp_show_create_routine(THD *thd, int type, sp_name *name)
   DBUG_ASSERT(type == TYPE_ENUM_PROCEDURE ||
               type == TYPE_ENUM_FUNCTION);
 
-  if (type == TYPE_ENUM_PROCEDURE)
+  /*
+    @todo: Consider using prelocking for this code as well. Currently
+    SHOW CREATE PROCEDURE/FUNCTION is a dirty read of the data
+    dictionary, i.e. takes no metadata locks.
+    It is "safe" to do as long as it doesn't affect the results
+    of the binary log or the query cache, which currently it does not.
+  */
+  if (sp_cache_routine(thd, type, name, FALSE, &sp))
+    DBUG_RETURN(TRUE);
+
+  if (sp == NULL || sp->show_create_routine(thd, type))
   {
     /*
-       SHOW CREATE PROCEDURE may require two instances of one sp_head
-       object when SHOW CREATE PROCEDURE is called for the procedure that
-       is being executed. Basically, there is no actual recursion, so we
-       increase the recursion limit for this statement (kind of hack).
-
-       SHOW CREATE FUNCTION does not require this because SHOW CREATE
-       statements are prohibitted within stored functions.
-     */
-
-    thd->variables.max_sp_recursion_depth++;
+      If we have insufficient privileges, pretend the routine
+      does not exist.
+    */
+    my_error(ER_SP_DOES_NOT_EXIST, MYF(0),
+             type == TYPE_ENUM_FUNCTION ? "FUNCTION" : "PROCEDURE",
+             name->m_name.str);
+    DBUG_RETURN(TRUE);
   }
 
-  if ((sp= sp_find_routine(thd, type, name, cache, FALSE)))
-    err_status= sp->show_create_routine(thd, type);
-
-  if (type == TYPE_ENUM_PROCEDURE)
-    thd->variables.max_sp_recursion_depth--;
-
-  DBUG_RETURN(err_status);
+  DBUG_RETURN(FALSE);
 }
 
 
@@ -1451,6 +1505,7 @@ bool sp_add_used_routine(Query_tables_list *prelocking_ctx, Query_arena *arena,
     my_hash_insert(&prelocking_ctx->sroutines, (uchar *)rn);
     prelocking_ctx->sroutines_list.link_in_list((uchar *)rn, (uchar **)&rn->next);
     rn->belong_to_view= belong_to_view;
+    rn->m_sp_cache_version= 0;
     return TRUE;
   }
   return FALSE;
@@ -1596,41 +1651,81 @@ void sp_update_stmt_used_routines(THD *thd, Query_tables_list *prelocking_ctx,
 }
 
 
+/**
+  A helper wrapper around sp_cache_routine() to use from
+  prelocking until 'sp_name' is eradicated as a class.
+*/
+
+int sp_cache_routine(THD *thd, Sroutine_hash_entry *rt,
+                     bool lookup_only, sp_head **sp)
+{
+  char qname_buff[NAME_LEN*2+1+1];
+  sp_name name(&rt->mdl_request.key, qname_buff);
+  MDL_key::enum_mdl_namespace mdl_type= rt->mdl_request.key.mdl_namespace();
+  int type= ((mdl_type == MDL_key::FUNCTION) ?
+             TYPE_ENUM_FUNCTION : TYPE_ENUM_PROCEDURE);
+
+  /*
+    Check that we have an MDL lock on this routine, unless it's a top-level
+    CALL. The assert below should be unambiguous: the first element
+    in sroutines_list has an MDL lock unless it's a top-level call, or a
+    trigger, but triggers can't occur here (see the preceding assert).
+  */
+  DBUG_ASSERT(rt->mdl_request.ticket ||
+              rt == (Sroutine_hash_entry*) thd->lex->sroutines_list.first);
+
+  return sp_cache_routine(thd, type, &name, lookup_only, sp);
+}
+
+
 /**
   Ensure that routine is present in cache by loading it from the mysql.proc
-  table if needed. Emit an appropriate error if there was a problem during
+  table if needed. If the routine is present but old, reload it.
+  Emit an appropriate error if there was a problem during
   loading.
 
   @param[in]  thd   Thread context.
   @param[in]  type  Type of object (TYPE_ENUM_FUNCTION or TYPE_ENUM_PROCEDURE).
   @param[in]  name  Name of routine.
+  @param[in]  lookup_only Only check that the routine is in the cache.
+                    If it's not, don't try to load. If it is present,
+                    but old, don't try to reload.
   @param[out] sp    Pointer to sp_head object for routine, NULL if routine was
-                    not found,
+                    not found.
 
   @retval 0      Either routine is found and was succesfully loaded into cache
                  or it does not exist.
   @retval non-0  Error while loading routine from mysql,proc table.
 */
 
-int sp_cache_routine(THD *thd, int type, sp_name *name, sp_head **sp)
+int sp_cache_routine(THD *thd, int type, sp_name *name,
+                     bool lookup_only, sp_head **sp)
 {
   int ret= 0;
+  sp_cache **spc= (type == TYPE_ENUM_FUNCTION ?
+                   &thd->sp_func_cache : &thd->sp_proc_cache);
 
   DBUG_ENTER("sp_cache_routine");
 
   DBUG_ASSERT(type == TYPE_ENUM_FUNCTION || type == TYPE_ENUM_PROCEDURE);
 
-  if (!(*sp= sp_cache_lookup((type == TYPE_ENUM_FUNCTION ?
-                              &thd->sp_func_cache : &thd->sp_proc_cache),
-                             name)))
+
+  *sp= sp_cache_lookup(spc, name);
+
+  if (lookup_only)
+    DBUG_RETURN(SP_OK);
+
+  if (*sp)
+  {
+    sp_cache_flush_obsolete(spc, sp);
+    if (*sp)
+      DBUG_RETURN(SP_OK);
+  }
+
+  switch ((ret= db_find_routine(thd, type, name, sp)))
   {
-    switch ((ret= db_find_routine(thd, type, name, sp)))
-    {
     case SP_OK:
-      if (type == TYPE_ENUM_FUNCTION)
-        sp_cache_insert(&thd->sp_func_cache, *sp);
-      else
-        sp_cache_insert(&thd->sp_proc_cache, *sp);
+      sp_cache_insert(spc, *sp);
       break;
     case SP_KEY_NOT_FOUND:
       ret= SP_OK;
@@ -1669,7 +1764,6 @@ int sp_cache_routine(THD *thd, int type, sp_name *name, sp_head **sp)
         my_error(ER_SP_PROC_TABLE_CORRUPT, MYF(0), n, ret);
       }
       break;
-    }
   }
   DBUG_RETURN(ret);
 }

sql/sp.h

@@ -43,7 +43,13 @@ sp_find_routine(THD *thd, int type, sp_name *name,
                 sp_cache **cp, bool cache_only);
 
 int
-sp_cache_routine(THD *thd, int type, sp_name *name, sp_head **sp);
+sp_cache_routine(THD *thd, Sroutine_hash_entry *rt,
+                 bool lookup_only, sp_head **sp);
+
+
+int
+sp_cache_routine(THD *thd, int type, sp_name *name,
+                 bool lookup_only, sp_head **sp);
 
 bool
 sp_exist_routines(THD *thd, TABLE_LIST *procs, bool any);
@@ -88,6 +94,16 @@ class Sroutine_hash_entry
     statement uses routine both via view and directly.
   */
   TABLE_LIST *belong_to_view;
+  /**
+    This is for prepared statement validation purposes.
+    A statement looks up and pre-loads all its stored functions
+    at prepare. Later on, if a function is gone from the cache,
+    execute may fail.
+    Remember the version of sp_head at prepare to be able to
+    invalidate the prepared statement at execute if it
+    changes.
+  */
+  ulong m_sp_cache_version;
 };
 
 

sql/sp_cache.cc

@@ -31,8 +31,6 @@ static ulong volatile Cversion= 0;
 class sp_cache
 {
 public:
-  ulong version;
-
   sp_cache();
   ~sp_cache();
 
@@ -48,25 +46,10 @@ class sp_cache
                                       namelen);
   }
 
-#ifdef NOT_USED
-  inline bool remove(char *name, uint namelen)
-  {
-    sp_head *sp= lookup(name, namelen);
-    if (sp)
-    {
-      hash_delete(&m_hashtable, (uchar *)sp);
-      return TRUE;
-    }
-    return FALSE;
-  }
-#endif 
-
-  inline void remove_all()
+  inline void remove(sp_head *sp)
   {
-    cleanup();
-    init();
+    my_hash_delete(&m_hashtable, (uchar *)sp);
   }
-
 private:
   void init();
   void cleanup();
@@ -129,8 +112,9 @@ void sp_cache_insert(sp_cache **cp, sp_head *sp)
   {
     if (!(c= new sp_cache()))
       return;                                   // End of memory error
-    c->version= Cversion;      // No need to lock when reading long variable
   }
+  /* Reading a ulong variable with no lock. */
+  sp->set_sp_cache_version(Cversion);
   DBUG_PRINT("info",("sp_cache: inserting: %.*s", (int) sp->m_qname.length,
                      sp->m_qname.str));
   c->insert(sp);
@@ -181,46 +165,34 @@ void sp_cache_invalidate()
 }
 
 
-/*
-  Remove out-of-date SPs from the cache. 
-  
-  SYNOPSIS
-    sp_cache_flush_obsolete()
-      cp  Cache to flush
+/**
+  Remove an out-of-date SP from the cache.
 
-  NOTE
-    This invalidates pointers to sp_head objects this thread uses.
-    In practice that means 'dont call this function when inside SP'.
+  @param[in] cp  Cache to flush
+  @param[in] sp  SP to remove.
+
+  @note This invalidates pointers to sp_head objects this thread
+  uses. In practice that means 'dont call this function when
+  inside SP'.
 */
 
-void sp_cache_flush_obsolete(sp_cache **cp)
+void sp_cache_flush_obsolete(sp_cache **cp, sp_head **sp)
 {
-  sp_cache *c= *cp;
-  if (c)
+  if ((*sp)->sp_cache_version() < Cversion && !(*sp)->is_invoked())
   {
-    ulong v;
-    v= Cversion;                 // No need to lock when reading long variable
-    if (c->version < v)
-    {
-      DBUG_PRINT("info",("sp_cache: deleting all functions"));
-      /* We need to delete all elements. */
-      c->remove_all();
-      c->version= v;
-    }
+    (*cp)->remove(*sp);
+    *sp= NULL;
   }
 }
 
 
 /**
-  Return the current version of the cache.
+  Return the current global version of the cache.
 */
 
-ulong sp_cache_version(sp_cache **cp)
+ulong sp_cache_version()
 {
-  sp_cache *c= *cp;
-  if (c)
-    return c->version;
-  return 0;
+  return Cversion;
 }
 
 
@@ -265,7 +237,6 @@ sp_cache::init()
 {
   my_hash_init(&m_hashtable, system_charset_info, 0, 0, 0,
                hash_get_key_for_sp_head, hash_free_sp_head, 0);
-  version= 0;
 }
 
 

sql/sp_cache.h

@@ -57,7 +57,7 @@ void sp_cache_clear(sp_cache **cp);
 void sp_cache_insert(sp_cache **cp, sp_head *sp);
 sp_head *sp_cache_lookup(sp_cache **cp, sp_name *name);
 void sp_cache_invalidate();
-void sp_cache_flush_obsolete(sp_cache **cp);
-ulong sp_cache_version(sp_cache **cp);
+void sp_cache_flush_obsolete(sp_cache **cp, sp_head **sp);
+ulong sp_cache_version();
 
 #endif /* _SP_CACHE_H_ */

sql/sp_head.cc

@@ -511,7 +511,10 @@ sp_head::operator delete(void *ptr, size_t size) throw()
 
 sp_head::sp_head()
   :Query_arena(&main_mem_root, INITIALIZED_FOR_SP),
-   m_flags(0), m_recursion_level(0), m_next_cached_sp(0),
+   m_flags(0),
+   m_sp_cache_version(0),
+   m_recursion_level(0),
+   m_next_cached_sp(0),
    m_cont_level(0)
 {
   const LEX_STRING str_reset= { NULL, 0 };
@@ -727,16 +730,6 @@ create_typelib(MEM_ROOT *mem_root, Create_field *field_def, List<String> *src)
 }
 
 
-int
-sp_head::create(THD *thd)
-{
-  DBUG_ENTER("sp_head::create");
-  DBUG_PRINT("info", ("type: %d name: %s params: %s body: %s",
-		      m_type, m_name.str, m_params.str, m_body.str));
-
-  DBUG_RETURN(sp_create_routine(thd, m_type, this));
-}
-
 sp_head::~sp_head()
 {
   DBUG_ENTER("sp_head::~sp_head");

sql/sp_head.h

@@ -175,7 +175,34 @@ class sp_head :private Query_arena
   LEX_STRING m_definer_user;
   LEX_STRING m_definer_host;
 
+  /**
+    Is this routine being executed?
+  */
+  bool is_invoked() const { return m_flags & IS_INVOKED; }
+
+  /**
+    Get the value of the SP cache version, as remembered
+    when the routine was inserted into the cache.
+  */
+  ulong sp_cache_version() const { return m_sp_cache_version; }
+
+  /** Set the value of the SP cache version.  */
+  void set_sp_cache_version(ulong version_arg)
+  {
+    m_sp_cache_version= version_arg;
+  }
 private:
+  /**
+    Version of the stored routine cache at the moment when the
+    routine was added to it. Is used only for functions and
+    procedures, not used for triggers or events.  When sp_head is
+    created, its version is 0. When it's added to the cache, the
+    version is assigned the global value 'Cversion'.
+    If later on Cversion is incremented, we know that the routine
+    is obsolete and should not be used --
+    sp_cache_flush_obsolete() will purge it.
+  */
+  ulong m_sp_cache_version;
   Stored_program_creation_ctx *m_creation_ctx;
 
 public:
@@ -263,9 +290,6 @@ class sp_head :private Query_arena
   void
   set_stmt_end(THD *thd);
 
-  int
-  create(THD *thd);
-
   virtual ~sp_head();
 
   /// Free memory

sql/sql_class.h

@@ -1271,7 +1271,7 @@ class Alter_table_prelocking_strategy : public Prelocking_strategy
 
 /**
   A context of open_tables() function, used to recover
-  from a failed open_table() attempt.
+  from a failed open_table() or open_routine() attempt.
 
   Implemented in sql_base.cc.
 */
@@ -1288,13 +1288,14 @@ class Open_table_context
   };
   Open_table_context(THD *thd);
 
-  bool recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *tables);
+  bool recover_from_failed_open(THD *thd, MDL_request *mdl_request,
+                                TABLE_LIST *table);
   bool request_backoff_action(enum_open_table_action action_arg);
 
   void add_request(MDL_request *request)
   { m_mdl_requests.push_front(request); }
 
-  bool can_recover_from_failed_open_table() const
+  bool can_recover_from_failed_open() const
   { return m_action != OT_NO_ACTION; }
   bool can_deadlock() const { return m_can_deadlock; }
 private:

sql/sql_prepare.cc

@@ -173,8 +173,6 @@ class Prepared_statement: public Statement
     SELECT_LEX and other classes).
   */
   MEM_ROOT main_mem_root;
-  /* Version of the stored functions cache at the time of prepare. */
-  ulong m_sp_cache_version;
 private:
   bool set_db(const char *db, uint db_length);
   bool set_parameters(String *expanded_query,
@@ -2138,9 +2136,6 @@ void mysqld_stmt_prepare(THD *thd, const char *packet, uint packet_length)
     DBUG_VOID_RETURN;
   }
 
-  sp_cache_flush_obsolete(&thd->sp_proc_cache);
-  sp_cache_flush_obsolete(&thd->sp_func_cache);
-
   thd->protocol= &thd->protocol_binary;
 
   if (stmt->prepare(packet, packet_length))
@@ -2419,6 +2414,13 @@ void reinit_stmt_before_use(THD *thd, LEX *lex)
   {
     tables->reinit_before_use(thd);
   }
+
+  /* Reset MDL tickets for procedures/functions */
+  for (Sroutine_hash_entry *rt=
+         (Sroutine_hash_entry*)thd->lex->sroutines_list.first;
+       rt; rt= rt->next)
+    rt->mdl_request.ticket= NULL;
+
   /*
     Cleanup of the special case of DELETE t1, t2 FROM t1, t2, t3 ...
     (multi-delete).  We do a full clean up, although at the moment all we
@@ -2512,9 +2514,6 @@ void mysqld_stmt_execute(THD *thd, char *packet_arg, uint packet_length)
   DBUG_PRINT("exec_query", ("%s", stmt->query()));
   DBUG_PRINT("info",("stmt: 0x%lx", (long) stmt));
 
-  sp_cache_flush_obsolete(&thd->sp_proc_cache);
-  sp_cache_flush_obsolete(&thd->sp_func_cache);
-
   open_cursor= test(flags & (ulong) CURSOR_TYPE_READ_ONLY);
 
   thd->protocol= &thd->protocol_binary;
@@ -2964,8 +2963,7 @@ Prepared_statement::Prepared_statement(THD *thd_arg)
   param_array(0),
   param_count(0),
   last_errno(0),
-  flags((uint) IS_IN_USE),
-  m_sp_cache_version(0)
+  flags((uint) IS_IN_USE)
 {
   init_sql_alloc(&main_mem_root, thd_arg->variables.query_alloc_block_size,
                   thd_arg->variables.query_prealloc_size);
@@ -3234,20 +3232,6 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len)
     init_stmt_after_parse(lex);
     state= Query_arena::PREPARED;
     flags&= ~ (uint) IS_IN_USE;
-    /*
-      This is for prepared statement validation purposes.
-      A statement looks up and pre-loads all its stored functions
-      at prepare. Later on, if a function is gone from the cache,
-      execute may fail.
-      Remember the cache version to be able to invalidate the prepared
-      statement at execute if it changes.
-      We only need to care about version of the stored functions cache:
-      if a prepared statement uses a stored procedure, it's indirect,
-      via a stored function. The only exception is SQLCOM_CALL,
-      but the latter one looks up the stored procedure each time
-      it's invoked, rather than once at prepare.
-    */
-    m_sp_cache_version= sp_cache_version(&thd->sp_func_cache);
 
     /* 
       Log COM_EXECUTE to the general log. Note, that in case of SQL
@@ -3588,13 +3572,12 @@ Prepared_statement::swap_prepared_statement(Prepared_statement *copy)
     is allocated in the old arena.
   */
   swap_variables(Item_param **, param_array, copy->param_array);
-  /* Swap flags: this is perhaps unnecessary */
-  swap_variables(uint, flags, copy->flags);
+  /* Don't swap flags: the copy has IS_SQL_PREPARE always set. */
+  /* swap_variables(uint, flags, copy->flags); */
   /* Swap names, the old name is allocated in the wrong memory root */
   swap_variables(LEX_STRING, name, copy->name);
   /* Ditto */
   swap_variables(char *, db, copy->db);
-  swap_variables(ulong, m_sp_cache_version, copy->m_sp_cache_version);
 
   DBUG_ASSERT(db_length == copy->db_length);
   DBUG_ASSERT(param_count == copy->param_count);
@@ -3653,19 +3636,6 @@ bool Prepared_statement::execute(String *expanded_query, bool open_cursor)
     return TRUE;
   }
 
-  /*
-    Reprepare the statement if we're using stored functions
-    and the version of the stored routines cache has changed.
-  */
-  if (lex->uses_stored_routines() &&
-      m_sp_cache_version != sp_cache_version(&thd->sp_func_cache) &&
-      thd->m_reprepare_observer &&
-      thd->m_reprepare_observer->report_error(thd))
-  {
-    return TRUE;
-  }
-
-
   /*
     For SHOW VARIABLES lex->result is NULL, as it's a non-SELECT
     command. For such queries we don't return an error and don't

sql/sql_show.cc

@@ -3237,7 +3237,6 @@ static int fill_schema_table_from_frm(THD *thd,TABLE *table,
 end_unlock:
   pthread_mutex_unlock(&LOCK_open);
 
-end:
   thd->mdl_context.release_lock(table_list.mdl_request.ticket);
   thd->clear_error();
   return res;

sql/sql_table.cc

@@ -21,6 +21,7 @@
 #include <myisam.h>
 #include <my_dir.h>
 #include "sp_head.h"
+#include "sp.h"
 #include "sql_trigger.h"
 #include "sql_show.h"
 #include "transaction.h"
@@ -5010,6 +5011,23 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
     trans_commit_implicit(thd);
     close_thread_tables(thd);
     table->table=0;				// For query cache
+
+    /*
+      If it is CHECK TABLE v1, v2, v3, and v1, v2, v3 are views, we will run
+      separate open_tables() for each CHECK TABLE argument.
+      Right now we do not have a separate method to reset the prelocking
+      state in the lex to the state after parsing, so each open will pollute
+      this state: add elements to lex->srotuines_list, TABLE_LISTs to
+      lex->query_tables. Below is a lame attempt to recover from this
+      pollution.
+      @todo: have a method to reset a prelocking context, or use separate
+      contexts for each open.
+    */
+    for (Sroutine_hash_entry *rt=
+           (Sroutine_hash_entry*)thd->lex->sroutines_list.first;
+         rt; rt= rt->next)
+      rt->mdl_request.ticket= NULL;
+
     if (protocol->write())
       goto err;
   }

sql/sql_update.cc

@@ -1117,10 +1117,6 @@ int mysql_multi_update_prepare(THD *thd)
     while ((item= it++))
       item->cleanup();
 
-    /* We have to cleanup translation tables of views. */
-    for (TABLE_LIST *tbl= table_list; tbl; tbl= tbl->next_global)
-      tbl->cleanup_items();
-
     /*
       To not to hog memory (as a result of the 
       unit->reinit_exec_mechanism() call below):