bk commit into 5.0 tree (dlenev:1.2008) BUG#11554

[dlenev]

Below is the list of changes that have just been committed into a local
5.0 repository of dlenev. When dlenev does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet
  1.2008 05/06/28 23:48:08 dlenev@stripped +7 -0
  Fix for bug #11554 "Server crashes on statement indirectly using
  non-cached function".
  
  Instead of opening mysql.proc table and then closing ALL tables open
  for each lookup of stored routine definition during building of table
  list for prelocking we should use the same TABLE instance for all these
  lookups and close only this instance at the end of the process.

  sql/sql_lex.h
    1.187 05/06/28 23:48:02 dlenev@stripped +0 -1
    Removed LEX::proc_table member which was not really used.

  sql/sql_lex.cc
    1.151 05/06/28 23:48:02 dlenev@stripped +1 -1
    Removed LEX::proc_table member which was not really used.

  sql/sql_base.cc
    1.260 05/06/28 23:48:02 dlenev@stripped +33 -2
    open_tables():
      Let us use the same TABLE instance for mysql.proc table for all lookups
      of stored routines definitions during building of table list for
      pre-locking. We also should unlock and close only this instance at
      the end of this process.

  sql/sp.h
    1.22 05/06/28 23:48:02 dlenev@stripped +1 -4
    sp_function_exists():
      Removed unused function.
    db_find_routine():
      Added explicit proc_table in/out parameter to be able use one
      TABLE instance corresponding to open mysql.proc table for several
      invocations of these functions which happen when we read stored
      routines definitions in open_tables().

  sql/sp.cc
    1.78 05/06/28 23:48:02 dlenev@stripped +81 -62
    sp_function_exists():
      Removed unused function.
    db_find_routine_aux():
      Now we use explicit in/out parameter for passing pointer to TABLE
      object for open mysql.proc table instead of using LEX::proc_table
      (It did not work anyway since there was no place where we were setting
      it to non-0 value). This also allows to get rid of opened parameter.
    db_find_routine()/sp_cache_routines():
      Added explicit proc_table in/out parameter to be able use one
      TABLE instance corresponding to open mysql.proc table for several
      invocations of these functions which happen when we read stored
      routines definitions in open_tables().

  mysql-test/t/sp-threads.test
    1.5 05/06/28 23:48:02 dlenev@stripped +19 -0
    Added test for bug #11554 "Server crashes on statement indirectly using
    non-cached function".

  mysql-test/r/sp-threads.result
    1.4 05/06/28 23:48:02 dlenev@stripped +9 -0
    Added test for bug #11554 "Server crashes on statement indirectly using
    non-cached function".

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	dlenev
# Host:	brandersnatch.localdomain
# Root:	/home/dlenev/src/mysql-5.0-bg11554

--- 1.259/sql/sql_base.cc	Mon Jun 27 17:02:36 2005
+++ 1.260/sql/sql_base.cc	Tue Jun 28 23:48:02 2005
@@ -1765,6 +1765,7 @@
   MEM_ROOT new_frm_mem;
   /* Also used for indicating that prelocking is need */
   TABLE_LIST **query_tables_last_own;
+  TABLE *proc_table= 0;
   DBUG_ENTER("open_tables");
   /*
     temporary mem_root for new .frm parsing.
@@ -1806,7 +1807,7 @@
   {
     TABLE_LIST **save_query_tables_last;
 
-    sp_cache_routines(thd, thd->lex);
+    sp_cache_routines(thd, thd->lex, &proc_table);
     save_query_tables_last= thd->lex->query_tables_last;
 
     DBUG_ASSERT(thd->lex->query_tables == *start);
@@ -1850,7 +1851,7 @@
             (tables->view->spfuns.records || tables->view->spprocs.records))
         {
           // FIXME We should catch recursion for both views and funcs here
-          sp_cache_routines(thd, tables->view);
+          sp_cache_routines(thd, tables->view, &proc_table);
 
           /* We have at least one table in TL here */
           if (!query_tables_last_own)
@@ -1937,6 +1938,36 @@
 
   if (query_tables_last_own)
     thd->lex->mark_as_requiring_prelocking(query_tables_last_own);
+
+  /*
+    To increase concurrency we have to unlock and close mysql.proc table
+    which we have opened for reading stored routine definitions. Note that
+    this table should not be open if we already in prelocked mode. We also
+    should do nothing if we are under LOCK TABLES.
+  */
+  DBUG_ASSERT(!(proc_table && thd->prelocked_mode));
+
+  if (proc_table && !thd->locked_tables)
+  {
+    TABLE **proc_table_ptr;
+    /*
+      Only one table should be locked by this moment - mysql.proc so it
+      is ok to unlock "all" tables.
+    */
+    DBUG_ASSERT(thd->lock->table_count == 1);
+    mysql_unlock_tables(thd, thd->lock);
+    thd->lock=0;
+    /* Find position of our mysql.proc instance in THD::open_tables list. */
+    for (proc_table_ptr= &thd->open_tables;
+         *proc_table_ptr != proc_table;
+         proc_table_ptr= &(*proc_table_ptr)->next)
+      {}
+    VOID(pthread_mutex_lock(&LOCK_open));
+    /* QQ: Should we do anything else here ? */
+    if (close_thread_table(thd, proc_table_ptr))
+      VOID(pthread_cond_broadcast(&COND_refresh));
+    VOID(pthread_mutex_unlock(&LOCK_open));
+  }
 
   DBUG_RETURN(result);
 }

--- 1.150/sql/sql_lex.cc	Fri Jun 17 23:27:21 2005
+++ 1.151/sql/sql_lex.cc	Tue Jun 28 23:48:02 2005
@@ -145,7 +145,7 @@
   lex->found_semicolon= 0;
   lex->safe_to_cache_query= 1;
   lex->time_zone_tables_used= 0;
-  lex->leaf_tables_insert= lex->proc_table= lex->query_tables= 0;
+  lex->leaf_tables_insert= lex->query_tables= 0;
   lex->query_tables_last= &lex->query_tables;
   lex->variables_used= 0;
   lex->select_lex.parent_lex= lex;

--- 1.186/sql/sql_lex.h	Fri Jun 24 22:48:06 2005
+++ 1.187/sql/sql_lex.h	Tue Jun 28 23:48:02 2005
@@ -734,7 +734,6 @@
     function)
   */
   TABLE_LIST **query_tables_last;
-  TABLE_LIST *proc_table; /* refer to mysql.proc if it was opened by VIEW */
   /* store original leaf_tables for INSERT SELECT and PS/SP */
   TABLE_LIST *leaf_tables_insert;
 

--- 1.3/mysql-test/r/sp-threads.result	Thu Jun  9 01:07:46 2005
+++ 1.4/mysql-test/r/sp-threads.result	Tue Jun 28 23:48:02 2005
@@ -55,3 +55,12 @@
 unlock tables;
 drop procedure bug11158;
 drop table t1, t2;
+drop function if exists bug11554;
+drop view if exists v1;
+create table t1 (i int);
+create function bug11554 () returns int return 1;
+create view v1 as select bug11554() as f;
+insert into t1 (select f from v1);
+drop function bug11554;
+drop table t1;
+drop view v1;

--- 1.4/mysql-test/t/sp-threads.test	Thu Jun  9 01:07:46 2005
+++ 1.5/mysql-test/t/sp-threads.test	Tue Jun 28 23:48:02 2005
@@ -112,6 +112,25 @@
 drop table t1, t2;
 
 #
+# BUG#11554: Server crashes on statement indirectly using non-cached function
+#
+--disable_warnings
+drop function if exists bug11554;
+drop view if exists v1;
+--enable_warnings
+create table t1 (i int);
+create function bug11554 () returns int return 1;
+create view v1 as select bug11554() as f;
+connection con2root;
+# This should not crash server
+insert into t1 (select f from v1);
+# Clean-up
+connection con1root;
+drop function bug11554;
+drop table t1;
+drop view v1;
+
+#
 # BUG#NNNN: New bug synopsis
 #
 #--disable_warnings

--- 1.77/sql/sp.cc	Tue May 31 20:36:27 2005
+++ 1.78/sql/sp.cc	Tue Jun 28 23:48:02 2005
@@ -61,20 +61,41 @@
 /* Tells what SP_DEFAULT_ACCESS should be mapped to */
 #define SP_DEFAULT_ACCESS_MAPPING SP_CONTAINS_SQL
 
-/* *opened=true means we opened ourselves */
+
+/*
+  Find row of mysql.proc table representing stored routine (also opens
+  and locks this table if needed).
+
+  SYNOPSIS
+    db_find_routine_aux()
+      thd        - thread context
+      type       - type of routine (TYPE_ENUM_PROCEDURE/...)
+      name       - name of routine
+      ltype      - type of lock which we should obtain for mysql.proc
+      proc_table - in/out parameter, pointer to TABLE object for open
+                   mysql.proc table, 0 if table was not open yet
+
+  NOTE
+    Caller of this function becames responsible for closing opened
+    mysql.proc table.
+
+  RETURN VALUE
+    SP_OPEN_TABLE_FAILED - error, we were unable to open mysql.proc table
+    SP_KEY_NOT_FOUND     - row for routine was not found
+    SP_OK                - row for routine was found and was stored in
+                           (*proc_table)->record[0] buffer
+*/
+
 static int
 db_find_routine_aux(THD *thd, int type, sp_name *name,
-		    enum thr_lock_type ltype, TABLE **tablep, bool *opened)
+		    enum thr_lock_type ltype, TABLE **proc_table)
 {
-  TABLE *table;
+  TABLE *table= *proc_table;
   byte key[NAME_LEN*2+4+1];	// db, name, optional key length type
   DBUG_ENTER("db_find_routine_aux");
   DBUG_PRINT("enter", ("type: %d name: %*s",
 		       type, name->m_name.length, name->m_name.str));
 
-  *opened= FALSE;
-  *tablep= 0;
-
   /*
     Speed up things if mysql.proc doesn't exists. mysql_proc_table_exists
     is set when we create or read stored procedure or on flush privileges.
@@ -82,15 +103,6 @@
   if (!mysql_proc_table_exists && ltype == TL_READ)
     DBUG_RETURN(SP_OPEN_TABLE_FAILED);
 
-  if (thd->lex->proc_table)
-    table= thd->lex->proc_table->table;
-  else
-  {
-    for (table= thd->open_tables ; table ; table= table->next)
-      if (strcmp(table->s->db, "mysql") == 0 &&
-          strcmp(table->s->table_name, "proc") == 0)
-        break;
-  }
   if (!table)
   {
     TABLE_LIST tables;
@@ -109,7 +121,7 @@
         mysql_proc_table_exists= 0;
       DBUG_RETURN(SP_OPEN_TABLE_FAILED);
     }
-    *opened= TRUE;
+    *proc_table= table;
   }
   mysql_proc_table_exists= 1;
 
@@ -137,20 +149,48 @@
   {
     DBUG_RETURN(SP_KEY_NOT_FOUND);
   }
-  *tablep= table;
 
   DBUG_RETURN(SP_OK);
 }
 
 
+/*
+  Find routine definition in mysql.proc table and create corresponding
+  sp_head object for it.
+
+  SYNOPSIS
+    db_find_routine()
+      thd        - thread context
+      type       - type of routine (TYPE_ENUM_PROCEDURE/...)
+      name       - name of routine
+      sphp       - out parameter in which pointer to created
+                   sp_head object is returned.
+      proc_table - If 0 then db_find_routine() is fully responsible for
+                   opening and closing mysql.proc table instance.
+                   If non-0 then serves as in/out parameter holding pointer
+                   to TABLE instance for open mysql.proc table (this pointer
+                   may be 0 if table is not open yet). In this case caller
+                   becomes responsible for closing this table.
+
+  NOTE
+    - This function may damage current LEX during execution, so it is good
+      idea to create temporary LEX and make it active before calling it.
+    - If "proc_table" parameter is 0 then this function will close not only
+      mysql.proc which it has opened but all tables opened by this thread.
+
+  RETURN VALUE
+    0     - Success
+    non-0 - Error (may be one of special codes like SP_KEY_NOT_FOUND)
+*/
+
 static int
-db_find_routine(THD *thd, int type, sp_name *name, sp_head **sphp)
+db_find_routine(THD *thd, int type, sp_name *name, sp_head **sphp,
+                TABLE **proc_table)
 {
   extern int yyparse(void *thd);
-  TABLE *table;
+  TABLE *table= proc_table ? *proc_table : 0;
   const char *params, *returns, *body;
   int ret;
-  bool opened;
   const char *definer;
   longlong created;
   longlong modified;
@@ -164,7 +204,7 @@
   DBUG_PRINT("enter", ("type: %d name: %*s",
 		       type, name->m_name.length, name->m_name.str));
 
-  ret= db_find_routine_aux(thd, type, name, TL_READ, &table, &opened);
+  ret= db_find_routine_aux(thd, type, name, TL_READ, &table);
   if (ret != SP_OK)
     goto done;
 
@@ -257,12 +297,6 @@
   chistics.comment.str= ptr;
   chistics.comment.length= length;
 
-  if (opened)
-  {
-    opened= FALSE;
-    close_thread_tables(thd, 0, 1);
-  }
-
   {
     String defstr;
     LEX *oldlex= thd->lex;
@@ -338,8 +372,11 @@
 
  done:
 
-  if (opened)
+  if (proc_table)
+    *proc_table= table;
+  else if (table)
     close_thread_tables(thd);
+
   DBUG_RETURN(ret);
 }
 
@@ -489,21 +526,21 @@
 static int
 db_drop_routine(THD *thd, int type, sp_name *name)
 {
-  TABLE *table;
+  TABLE *table= 0;
   int ret;
   bool opened;
   DBUG_ENTER("db_drop_routine");
   DBUG_PRINT("enter", ("type: %d name: %*s",
 		       type, name->m_name.length, name->m_name.str));
 
-  ret= db_find_routine_aux(thd, type, name, TL_WRITE, &table, &opened);
+  ret= db_find_routine_aux(thd, type, name, TL_WRITE, &table);
   if (ret == SP_OK)
   {
     if (table->file->delete_row(table->record[0]))
       ret= SP_DELETE_ROW_FAILED;
   }
 
-  if (opened)
+  if (table)
     close_thread_tables(thd);
   DBUG_RETURN(ret);
 }
@@ -512,14 +549,14 @@
 static int
 db_update_routine(THD *thd, int type, sp_name *name, st_sp_chistics *chistics)
 {
-  TABLE *table;
+  TABLE *table= 0;
   int ret;
   bool opened;
   DBUG_ENTER("db_update_routine");
   DBUG_PRINT("enter", ("type: %d name: %*s",
 		       type, name->m_name.length, name->m_name.str));
 
-  ret= db_find_routine_aux(thd, type, name, TL_WRITE, &table, &opened);
+  ret= db_find_routine_aux(thd, type, name, TL_WRITE, &table);
   if (ret == SP_OK)
   {
     store_record(table,record[1]);
@@ -538,7 +575,7 @@
     if ((table->file->update_row(table->record[1],table->record[0])))
       ret= SP_WRITE_ROW_FAILED;
   }
-  if (opened)
+  if (table)
     close_thread_tables(thd);
   DBUG_RETURN(ret);
 }
@@ -819,7 +856,7 @@
 
   if (!(sp= sp_cache_lookup(&thd->sp_proc_cache, name)) && !cache_only)
   {
-    if (db_find_routine(thd, TYPE_ENUM_PROCEDURE, name, &sp) == SP_OK)
+    if (db_find_routine(thd, TYPE_ENUM_PROCEDURE, name, &sp, NULL) == SP_OK)
       sp_cache_insert(&thd->sp_proc_cache, sp);
   }
 
@@ -972,7 +1009,7 @@
   if (!(sp= sp_cache_lookup(&thd->sp_func_cache, name)) &&
       !cache_only)
   {
-    if (db_find_routine(thd, TYPE_ENUM_FUNCTION, name, &sp) != SP_OK)
+    if (db_find_routine(thd, TYPE_ENUM_FUNCTION, name, &sp, NULL) != SP_OK)
       sp= NULL;
     else
       sp_cache_insert(&thd->sp_func_cache, sp);
@@ -1052,26 +1089,6 @@
 }
 
 
-bool
-sp_function_exists(THD *thd, sp_name *name)
-{
-  TABLE *table;
-  bool ret= FALSE;
-  bool opened= FALSE;
-  DBUG_ENTER("sp_function_exists");
-
-  if (sp_cache_lookup(&thd->sp_func_cache, name) ||
-      db_find_routine_aux(thd, TYPE_ENUM_FUNCTION,
-			  name, TL_READ,
-			  &table, &opened) == SP_OK)
-    ret= TRUE;
-  if (opened)
-    close_thread_tables(thd, 0, 1);
-  thd->clear_error();
-  DBUG_RETURN(ret);
-}
-
-
 byte *
 sp_lex_sp_key(const byte *ptr, uint *plen, my_bool first)
 {
@@ -1132,8 +1149,12 @@
 
   SYNOPSIS
     sp_cache_routines()
-      thd - thread context
-      lex - LEX representing query
+      thd        - thread context
+      lex        - LEX representing query
+      proc_table - in/out parameter holding pointer to TABLE object
+                   for open mysql.proc table (holds zero if table
+                   is not open yet). Caller becomes responsible
+                   for closing this table.
 
   NOTE
     If some function is missing this won't be reported here.
@@ -1150,7 +1171,7 @@
 */
 
 void
-sp_cache_routines(THD *thd, LEX *lex)
+sp_cache_routines(THD *thd, LEX *lex, TABLE **proc_table)
 {
   bool routines_added= TRUE;
 
@@ -1179,8 +1200,6 @@
           LEX *newlex= new st_lex;
 
           thd->lex= newlex;
-          /* Pass hint pointer to mysql.proc table */
-          newlex->proc_table= oldlex->proc_table;
           newlex->current_select= NULL;
           name.m_name.str= strchr(name.m_qname.str, '.');
           name.m_db.length= name.m_name.str - name.m_qname.str;
@@ -1189,7 +1208,7 @@
           name.m_name.str+= 1;
           name.m_name.length= name.m_qname.length - name.m_db.length - 1;
 
-          if (db_find_routine(thd, type, &name, &sp) == SP_OK)
+          if (db_find_routine(thd, type, &name, &sp, proc_table) == SP_OK)
           {
             if (type == TYPE_ENUM_FUNCTION)
               sp_cache_insert(&thd->sp_func_cache, sp);

--- 1.21/sql/sp.h	Tue May 31 20:36:27 2005
+++ 1.22/sql/sp.h	Tue Jun 28 23:48:02 2005
@@ -74,9 +74,6 @@
 int
 sp_show_status_function(THD *thd, const char *wild);
 
-bool
-sp_function_exists(THD *thd, sp_name *name);
-
 
 /*
  *  For precaching of functions and procedures
@@ -86,7 +83,7 @@
 bool
 sp_merge_hash(HASH *dst, HASH *src);
 void
-sp_cache_routines(THD *thd, LEX *lex);
+sp_cache_routines(THD *thd, LEX *lex, TABLE **proc_table);
 
 
 //