From cab73a600927b82470d71b96f6d01232e0cf6e87 Mon Sep 17 00:00:00 2001
From: unknown <kroki@mysql.com>
Date: Fri, 7 Apr 2006 15:30:40 +0400
Subject: Bug#15933: max_used_connections is wrong after FLUSH STATUS if
 connections are cached

After FLUSH STATUS max_used_connections was reset to 0, and haven't
been updated while cached threads were reused, until the moment a new
thread was created.

The first suggested fix from original bug report was implemented:

  a) On flushing the status, set max_used_connections to
  threads_connected, not to 0.

  b) Check if it is necessary to increment max_used_connections when
  taking a thread from the cache as well as when creating new threads


mysql-test/r/status.result:
  Add result for bug#15933.
mysql-test/t/status.test:
  Add test case for bug#15933.
  Fixed typo.
sql/mysql_priv.h:
  Add declaration of refresh_status(), which is now external.
sql/mysqld.cc:
  Remove start_cached_thread() (code moved directly into create_new_thread()).
  Add comment for create_new_thread ().
  In create_new_thread() update max_used_connections when creating new thread
  and when reusing the cached one.
  Move refresh_status() from sql/sql_parse.cc here, on refresh set
  max_used_connections to the current number of connections.
sql/sql_parse.cc:
  refresh_status() moved to sql/mysqld.cc.
---
 mysql-test/r/status.result | 20 +++++++++++++
 mysql-test/t/status.test   | 65 ++++++++++++++++++++++++++++++++++++++++-
 sql/mysql_priv.h           |  1 +
 sql/mysqld.cc              | 73 ++++++++++++++++++++++++++++++++++++----------
 sql/sql_parse.cc           | 22 --------------
 5 files changed, 142 insertions(+), 39 deletions(-)

diff --git a/mysql-test/r/status.result b/mysql-test/r/status.result
index 5461e4dd563..4e5489191c9 100644
--- a/mysql-test/r/status.result
+++ b/mysql-test/r/status.result
@@ -23,3 +23,23 @@ select 1;
 show status like 'last_query_cost';
 Variable_name	Value
 Last_query_cost	0.000000
+FLUSH STATUS;
+SHOW STATUS LIKE 'max_used_connections';
+Variable_name	Value
+Max_used_connections	3
+SET @save_thread_cache_size=@@thread_cache_size;
+SET GLOBAL thread_cache_size=3;
+SHOW STATUS LIKE 'max_used_connections';
+Variable_name	Value
+Max_used_connections	5
+FLUSH STATUS;
+SHOW STATUS LIKE 'max_used_connections';
+Variable_name	Value
+Max_used_connections	4
+SHOW STATUS LIKE 'max_used_connections';
+Variable_name	Value
+Max_used_connections	5
+SHOW STATUS LIKE 'max_used_connections';
+Variable_name	Value
+Max_used_connections	6
+SET GLOBAL thread_cache_size=@save_thread_cache_size;
diff --git a/mysql-test/t/status.test b/mysql-test/t/status.test
index 929a0cb5877..79a46a38550 100644
--- a/mysql-test/t/status.test
+++ b/mysql-test/t/status.test
@@ -39,8 +39,71 @@ drop table t1;
 # End of 4.1 tests
 
 #
-# lost_query_cost
+# last_query_cost
 #
 
 select 1;
 show status like 'last_query_cost';
+
+#
+# Test for Bug #15933 max_used_connections is wrong after FLUSH STATUS
+# if connections are cached
+#
+#
+# The first suggested fix from the bug report was chosen
+# (see http://bugs.mysql.com/bug.php?id=15933):
+#
+#   a) On flushing the status, set max_used_connections to
+#   threads_connected, not to 0.
+#
+#   b) Check if it is necessary to increment max_used_connections when
+#   taking a thread from the cache as well as when creating new threads
+#
+
+# Previous test uses con1, con2.  If we disconnect them, there will be
+# a race on when exactly this will happen, so we better leave them as
+# is.
+connection default;
+
+# Reset max_used_connections from previous tests.
+FLUSH STATUS;
+SHOW STATUS LIKE 'max_used_connections';
+
+# Save original setting.
+SET @save_thread_cache_size=@@thread_cache_size;
+SET GLOBAL thread_cache_size=3;
+
+connect (con3,localhost,root,,);
+connect (con4,localhost,root,,);
+
+connection con3;
+disconnect con4;
+
+# Check that max_used_connections still reflects maximum value.
+SHOW STATUS LIKE 'max_used_connections';
+
+# Check that after flush max_used_connections equals to current number
+# of connections.
+FLUSH STATUS;
+SHOW STATUS LIKE 'max_used_connections';
+
+# Check that max_used_connections is updated when cached thread is
+# reused...
+connect (con4,localhost,root,,);
+SHOW STATUS LIKE 'max_used_connections';
+
+# ...and when new thread is created.
+connect (con5,localhost,root,,);
+SHOW STATUS LIKE 'max_used_connections';
+
+# Restore original setting.
+connection default;
+SET GLOBAL thread_cache_size=@save_thread_cache_size;
+
+disconnect con5;
+disconnect con4;
+disconnect con3;
+disconnect con2;
+disconnect con1;
+
+# End of 5.0 tests
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index ca7801039c5..03121383d8a 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -1097,6 +1097,7 @@ File open_binlog(IO_CACHE *log, const char *log_file_name,
 
 /* mysqld.cc */
 extern void MYSQLerror(const char*);
+void refresh_status(THD *thd);
 
 /* item_func.cc */
 extern bool check_reserved_words(LEX_STRING *name);
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index e9ff220a6a1..81b5be42a69 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -1639,17 +1639,6 @@ void end_thread(THD *thd, bool put_in_cache)
 }
 
 
-/* Start a cached thread. LOCK_thread_count is locked on entry */
-
-static void start_cached_thread(THD *thd)
-{
-  thread_cache.append(thd);
-  wake_thread++;
-  thread_count++;
-  pthread_cond_signal(&COND_thread_cache);
-}
-
-
 void flush_thread_cache()
 {
   (void) pthread_mutex_lock(&LOCK_thread_count);
@@ -3780,6 +3769,25 @@ static bool read_init_file(char *file_name)
 
 
 #ifndef EMBEDDED_LIBRARY
+/*
+  Create new thread to handle incoming connection.
+
+  SYNOPSIS
+    create_new_thread()
+      thd in/out    Thread handle of future thread.
+
+  DESCRIPTION
+    This function will create new thread to handle the incoming
+    connection.  If there are idle cached threads one will be used.
+    'thd' will be pushed into 'threads'.
+
+    In single-threaded mode (#define ONE_THREAD) connection will be
+    handled inside this function.
+
+  RETURN VALUE
+    none
+*/
+
 static void create_new_thread(THD *thd)
 {
   DBUG_ENTER("create_new_thread");
@@ -3803,11 +3811,12 @@ static void create_new_thread(THD *thd)
   thd->real_id=pthread_self();			// Keep purify happy
 
   /* Start a new thread to handle connection */
+  thread_count++;
+
 #ifdef ONE_THREAD
   if (test_flags & TEST_NO_THREADS)		// For debugging under Linux
   {
     thread_cache_size=0;			// Safety
-    thread_count++;
     threads.append(thd);
     thd->real_id=pthread_self();
     (void) pthread_mutex_unlock(&LOCK_thread_count);
@@ -3816,18 +3825,20 @@ static void create_new_thread(THD *thd)
   else
 #endif
   {
+    if (thread_count-delayed_insert_threads > max_used_connections)
+      max_used_connections=thread_count-delayed_insert_threads;
+
     if (cached_thread_count > wake_thread)
     {
-      start_cached_thread(thd);
+      thread_cache.append(thd);
+      wake_thread++;
+      pthread_cond_signal(&COND_thread_cache);
     }
     else
     {
       int error;
-      thread_count++;
       thread_created++;
       threads.append(thd);
-      if (thread_count-delayed_insert_threads > max_used_connections)
-        max_used_connections=thread_count-delayed_insert_threads;
       DBUG_PRINT("info",(("creating thread %d"), thd->thread_id));
       thd->connect_time = time(NULL);
       if ((error=pthread_create(&thd->real_id,&connection_attrib,
@@ -7393,6 +7404,36 @@ static void create_pid_file()
 }
 
 
+/* Clear most status variables */
+void refresh_status(THD *thd)
+{
+  pthread_mutex_lock(&LOCK_status);
+
+  /* We must update the global status before cleaning up the thread */
+  add_to_status(&global_status_var, &thd->status_var);
+  bzero((char*) &thd->status_var, sizeof(thd->status_var));
+
+  for (struct show_var_st *ptr=status_vars; ptr->name; ptr++)
+  {
+    if (ptr->type == SHOW_LONG)
+      *(ulong*) ptr->value= 0;
+  }
+  /* Reset the counters of all key caches (default and named). */
+  process_key_caches(reset_key_cache_counters);
+  pthread_mutex_unlock(&LOCK_status);
+
+  /*
+    Set max_used_connections to the number of currently open
+    connections.  Lock LOCK_thread_count out of LOCK_status to avoid
+    deadlocks.  Status reset becomes not atomic, but status data is
+    not exact anyway.
+  */
+  pthread_mutex_lock(&LOCK_thread_count);
+  max_used_connections= thread_count-delayed_insert_threads;
+  pthread_mutex_unlock(&LOCK_thread_count);
+}
+
+
 /*****************************************************************************
   Instantiate have_xyx for missing storage engines
 *****************************************************************************/
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index f9d04fc873e..114673430e2 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -74,7 +74,6 @@ static void decrease_user_connections(USER_CONN *uc);
 static bool check_db_used(THD *thd,TABLE_LIST *tables);
 static bool check_multi_update_lock(THD *thd);
 static void remove_escape(char *name);
-static void refresh_status(THD *thd);
 static bool append_file_to_dir(THD *thd, const char **filename_ptr,
 			       const char *table_name);
 
@@ -6709,27 +6708,6 @@ void kill_one_thread(THD *thd, ulong id, bool only_kill_query)
 }
 
 
-/* Clear most status variables */
-
-static void refresh_status(THD *thd)
-{
-  pthread_mutex_lock(&LOCK_status);
-
-  /* We must update the global status before cleaning up the thread */
-  add_to_status(&global_status_var, &thd->status_var);
-  bzero((char*) &thd->status_var, sizeof(thd->status_var));
-
-  for (struct show_var_st *ptr=status_vars; ptr->name; ptr++)
-  {
-    if (ptr->type == SHOW_LONG)
-      *(ulong*) ptr->value= 0;
-  }
-  /* Reset the counters of all key caches (default and named). */
-  process_key_caches(reset_key_cache_counters);
-  pthread_mutex_unlock(&LOCK_status);
-}
-
-
 	/* If pointer is not a null pointer, append filename to it */
 
 static bool append_file_to_dir(THD *thd, const char **filename_ptr,
-- 
cgit v1.2.1


From 148cf113e590e4f465c5c1c608ba4cf848abc388 Mon Sep 17 00:00:00 2001
From: unknown <pem@mysql.com>
Date: Fri, 7 Apr 2006 16:53:15 +0200
Subject: Renaming sp_pcontext members and methods; less cryptic and more
 consistent. Also added comments, and fixing some coding style (mostly in
 comments too). There are no functional changes, so no tests or documentation
 needed. (This was originally part of a bugfix, but it was decided to not
 include this  in that patch; instead it's done separately.)

sql/sp_head.cc:
  Renaming sp_pcontext members and methods; less cryptic and more consistent.
sql/sp_head.h:
  Renaming sp_pcontext members and methods; less cryptic and more consistent.
sql/sp_pcontext.cc:
  Renaming sp_pcontext members and methods; less cryptic and more consistent.
  Also added comments, and fixing some coding style (mostly in comments too).
sql/sp_pcontext.h:
  Renaming sp_pcontext members and methods; less cryptic and more consistent.
  Also added comments, and fixing some coding style (mostly in comments too).
sql/sp_rcontext.cc:
  Renaming sp_pcontext members and methods; less cryptic and more consistent.
sql/sp_rcontext.h:
  Renaming sp_pcontext members and methods; less cryptic and more consistent.
sql/sql_yacc.yy:
  Renaming sp_pcontext members and methods; less cryptic and more consistent.
---
 sql/sp_head.cc     |  22 ++++----
 sql/sp_head.h      |   6 +-
 sql/sp_pcontext.cc | 161 +++++++++++++++++++++++++++--------------------------
 sql/sp_pcontext.h  | 130 +++++++++++++++++++++++++-----------------
 sql/sp_rcontext.cc |  24 ++++----
 sql/sp_rcontext.h  |   8 +--
 sql/sql_yacc.yy    |  86 ++++++++++++++--------------
 7 files changed, 233 insertions(+), 204 deletions(-)

diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index bba9479c8f3..15d621b1d6d 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -1214,7 +1214,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount,
   DBUG_ENTER("sp_head::execute_function");
   DBUG_PRINT("info", ("function %s", m_name.str));
 
-  params = m_pcont->context_pvars();
+  params = m_pcont->context_var_count();
 
   /*
     Check that the function is called with all specified arguments.
@@ -1391,7 +1391,7 @@ bool
 sp_head::execute_procedure(THD *thd, List<Item> *args)
 {
   bool err_status= FALSE;
-  uint params = m_pcont->context_pvars();
+  uint params = m_pcont->context_var_count();
   sp_rcontext *save_spcont, *octx;
   sp_rcontext *nctx = NULL;
   DBUG_ENTER("sp_head::execute_procedure");
@@ -1443,15 +1443,15 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
     for (uint i= 0 ; i < params ; i++)
     {
       Item *arg_item= it_args++;
-      sp_pvar_t *pvar= m_pcont->find_pvar(i);
+      sp_variable_t *spvar= m_pcont->find_variable(i);
 
       if (!arg_item)
         break;
 
-      if (!pvar)
+      if (!spvar)
         continue;
 
-      if (pvar->mode != sp_param_in)
+      if (spvar->mode != sp_param_in)
       {
         if (!arg_item->is_splocal() && !item_is_user_var(arg_item))
         {
@@ -1461,7 +1461,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
         }
       }
 
-      if (pvar->mode == sp_param_out)
+      if (spvar->mode == sp_param_out)
       {
         Item_null *null_item= new Item_null();
 
@@ -1521,9 +1521,9 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
       if (!arg_item)
         break;
 
-      sp_pvar_t *pvar= m_pcont->find_pvar(i);
+      sp_variable_t *spvar= m_pcont->find_variable(i);
 
-      if (pvar->mode == sp_param_in)
+      if (spvar->mode == sp_param_in)
         continue;
 
       if (arg_item->is_splocal())
@@ -2388,7 +2388,7 @@ sp_instr_set::print(String *str)
 {
   /* set name@offset ... */
   int rsrv = SP_INSTR_UINT_MAXLEN+6;
-  sp_pvar_t *var = m_ctx->find_pvar(m_offset);
+  sp_variable_t *var = m_ctx->find_variable(m_offset);
 
   /* 'var' should always be non-null, but just in case... */
   if (var)
@@ -3004,8 +3004,8 @@ sp_instr_cfetch::execute(THD *thd, uint *nextp)
 void
 sp_instr_cfetch::print(String *str)
 {
-  List_iterator_fast<struct sp_pvar> li(m_varlist);
-  sp_pvar_t *pv;
+  List_iterator_fast<struct sp_variable> li(m_varlist);
+  sp_variable_t *pv;
   LEX_STRING n;
   my_bool found= m_ctx->find_cursor(m_cursor, &n);
   /* cfetch name@offset vars... */
diff --git a/sql/sp_head.h b/sql/sp_head.h
index 64d9167fb17..17a5d1ae528 100644
--- a/sql/sp_head.h
+++ b/sql/sp_head.h
@@ -44,7 +44,7 @@ class sp_instr;
 class sp_instr_opt_meta;
 class sp_instr_jump_if_not;
 struct sp_cond_type;
-struct sp_pvar;
+struct sp_variable;
 
 class sp_name : public Sql_alloc
 {
@@ -1074,7 +1074,7 @@ public:
 
   virtual void print(String *str);
 
-  void add_to_varlist(struct sp_pvar *var)
+  void add_to_varlist(struct sp_variable *var)
   {
     m_varlist.push_back(var);
   }
@@ -1082,7 +1082,7 @@ public:
 private:
 
   uint m_cursor;
-  List<struct sp_pvar> m_varlist;
+  List<struct sp_variable> m_varlist;
 
 }; // class sp_instr_cfetch : public sp_instr
 
diff --git a/sql/sp_pcontext.cc b/sql/sp_pcontext.cc
index f69053a7c88..448df908a32 100644
--- a/sql/sp_pcontext.cc
+++ b/sql/sp_pcontext.cc
@@ -27,10 +27,10 @@
 #include "sp_head.h"
 
 /*
- * Sanity check for SQLSTATEs. Will not check if it's really an existing
- * state (there are just too many), but will check length and bad characters.
- * Returns TRUE if it's ok, FALSE if it's bad.
- */
+  Sanity check for SQLSTATEs. Will not check if it's really an existing
+  state (there are just too many), but will check length and bad characters.
+  Returns TRUE if it's ok, FALSE if it's bad.
+*/
 bool
 sp_cond_check(LEX_STRING *sqlstate)
 {
@@ -51,25 +51,25 @@ sp_cond_check(LEX_STRING *sqlstate)
 }
 
 sp_pcontext::sp_pcontext(sp_pcontext *prev)
-  :Sql_alloc(), m_total_pvars(0), m_csubsize(0), m_hsubsize(0),
-   m_handlers(0), m_parent(prev), m_pboundary(0)
+  :Sql_alloc(), m_max_var_index(0), m_max_cursor_index(0), m_max_handler_index(0),
+   m_context_handlers(0), m_parent(prev), m_pboundary(0)
 {
-  VOID(my_init_dynamic_array(&m_pvar, sizeof(sp_pvar_t *), 16, 8));
+  VOID(my_init_dynamic_array(&m_vars, sizeof(sp_variable_t *), 16, 8));
   VOID(my_init_dynamic_array(&m_case_expr_id_lst, sizeof(int), 16, 8));
-  VOID(my_init_dynamic_array(&m_cond, sizeof(sp_cond_type_t *), 16, 8));
-  VOID(my_init_dynamic_array(&m_cursor, sizeof(LEX_STRING), 16, 8));
-  VOID(my_init_dynamic_array(&m_handler, sizeof(sp_cond_type_t *), 16, 8));
+  VOID(my_init_dynamic_array(&m_conds, sizeof(sp_cond_type_t *), 16, 8));
+  VOID(my_init_dynamic_array(&m_cursors, sizeof(LEX_STRING), 16, 8));
+  VOID(my_init_dynamic_array(&m_handlers, sizeof(sp_cond_type_t *), 16, 8));
   m_label.empty();
   m_children.empty();
   if (!prev)
   {
-    m_poffset= m_coffset= 0;
+    m_var_offset= m_cursor_offset= 0;
     m_num_case_exprs= 0;
   }
   else
   {
-    m_poffset= prev->m_poffset + prev->m_total_pvars;
-    m_coffset= prev->current_cursors();
+    m_var_offset= prev->m_var_offset + prev->m_max_var_index;
+    m_cursor_offset= prev->current_cursor_count();
     m_num_case_exprs= prev->get_num_case_exprs();
   }
 }
@@ -85,11 +85,11 @@ sp_pcontext::destroy()
 
   m_children.empty();
   m_label.empty();
-  delete_dynamic(&m_pvar);
+  delete_dynamic(&m_vars);
   delete_dynamic(&m_case_expr_id_lst);
-  delete_dynamic(&m_cond);
-  delete_dynamic(&m_cursor);
-  delete_dynamic(&m_handler);
+  delete_dynamic(&m_conds);
+  delete_dynamic(&m_cursors);
+  delete_dynamic(&m_handlers);
 }
 
 sp_pcontext *
@@ -105,15 +105,15 @@ sp_pcontext::push_context()
 sp_pcontext *
 sp_pcontext::pop_context()
 {
-  m_parent->m_total_pvars= m_parent->m_total_pvars + m_total_pvars;
+  m_parent->m_max_var_index+= m_max_var_index;
 
-  uint submax= max_handlers();
-  if (submax > m_parent->m_hsubsize)
-    m_parent->m_hsubsize= submax;
+  uint submax= max_handler_index();
+  if (submax > m_parent->m_max_handler_index)
+    m_parent->m_max_handler_index= submax;
 
-  submax= max_cursors();
-  if (submax > m_parent->m_csubsize)
-    m_parent->m_csubsize= submax;
+  submax= max_cursor_index();
+  if (submax > m_parent->m_max_cursor_index)
+    m_parent->m_max_cursor_index= submax;
 
   if (m_num_case_exprs > m_parent->m_num_case_exprs)
     m_parent->m_num_case_exprs= m_num_case_exprs;
@@ -130,12 +130,12 @@ sp_pcontext::diff_handlers(sp_pcontext *ctx, bool exclusive)
 
   while (pctx && pctx != ctx)
   {
-    n+= pctx->m_handlers;
+    n+= pctx->m_context_handlers;
     last_ctx= pctx;
     pctx= pctx->parent_context();
   }
   if (pctx)
-    return (exclusive && last_ctx ? n - last_ctx->m_handlers : n);
+    return (exclusive && last_ctx ? n - last_ctx->m_context_handlers : n);
   return 0;			// Didn't find ctx
 }
 
@@ -148,32 +148,33 @@ sp_pcontext::diff_cursors(sp_pcontext *ctx, bool exclusive)
 
   while (pctx && pctx != ctx)
   {
-    n+= pctx->m_cursor.elements;
+    n+= pctx->m_cursors.elements;
     last_ctx= pctx;
     pctx= pctx->parent_context();
   }
   if (pctx)
-    return  (exclusive && last_ctx ? n - last_ctx->m_cursor.elements : n);
+    return  (exclusive && last_ctx ? n - last_ctx->m_cursors.elements : n);
   return 0;			// Didn't find ctx
 }
 
-/* This does a linear search (from newer to older variables, in case
-** we have shadowed names).
-** It's possible to have a more efficient allocation and search method,
-** but it might not be worth it. The typical number of parameters and
-** variables will in most cases be low (a handfull).
-** ...and, this is only called during parsing.
+/*
+  This does a linear search (from newer to older variables, in case
+  we have shadowed names).
+  It's possible to have a more efficient allocation and search method,
+  but it might not be worth it. The typical number of parameters and
+  variables will in most cases be low (a handfull).
+  ...and, this is only called during parsing.
 */
-sp_pvar_t *
-sp_pcontext::find_pvar(LEX_STRING *name, my_bool scoped)
+sp_variable_t *
+sp_pcontext::find_variable(LEX_STRING *name, my_bool scoped)
 {
-  uint i= m_pvar.elements - m_pboundary;
+  uint i= m_vars.elements - m_pboundary;
 
   while (i--)
   {
-    sp_pvar_t *p;
+    sp_variable_t *p;
 
-    get_dynamic(&m_pvar, (gptr)&p, i);
+    get_dynamic(&m_vars, (gptr)&p, i);
     if (my_strnncoll(system_charset_info,
 		     (const uchar *)name->str, name->length,
 		     (const uchar *)p->name.str, p->name.length) == 0)
@@ -182,7 +183,7 @@ sp_pcontext::find_pvar(LEX_STRING *name, my_bool scoped)
     }
   }
   if (!scoped && m_parent)
-    return m_parent->find_pvar(name, scoped);
+    return m_parent->find_variable(name, scoped);
   return NULL;
 }
 
@@ -192,40 +193,40 @@ sp_pcontext::find_pvar(LEX_STRING *name, my_bool scoped)
   - When evaluating parameters at the beginning, and setting out parameters
     at the end, of invokation. (Top frame only, so no recursion then.)
   - For printing of sp_instr_set. (Debug mode only.)
- */
-sp_pvar_t *
-sp_pcontext::find_pvar(uint offset)
+*/
+sp_variable_t *
+sp_pcontext::find_variable(uint offset)
 {
-  if (m_poffset <= offset && offset < m_poffset + m_pvar.elements)
+  if (m_var_offset <= offset && offset < m_var_offset + m_vars.elements)
   {                           // This frame
-    sp_pvar_t *p;
+    sp_variable_t *p;
 
-    get_dynamic(&m_pvar, (gptr)&p, offset - m_poffset);
+    get_dynamic(&m_vars, (gptr)&p, offset - m_var_offset);
     return p;
   }
   if (m_parent)
-    return m_parent->find_pvar(offset); // Some previous frame
+    return m_parent->find_variable(offset); // Some previous frame
   return NULL;                  // index out of bounds
 }
 
-sp_pvar_t *
-sp_pcontext::push_pvar(LEX_STRING *name, enum enum_field_types type,
-		       sp_param_mode_t mode)
+sp_variable_t *
+sp_pcontext::push_variable(LEX_STRING *name, enum enum_field_types type,
+                           sp_param_mode_t mode)
 {
-  sp_pvar_t *p= (sp_pvar_t *)sql_alloc(sizeof(sp_pvar_t));
+  sp_variable_t *p= (sp_variable_t *)sql_alloc(sizeof(sp_variable_t));
 
   if (!p)
     return NULL;
 
-  ++m_total_pvars;
+  ++m_max_var_index;
 
   p->name.str= name->str;
   p->name.length= name->length;
   p->type= type;
   p->mode= mode;
-  p->offset= current_pvars();
+  p->offset= current_var_count();
   p->dflt= NULL;
-  insert_dynamic(&m_pvar, (gptr)&p);
+  insert_dynamic(&m_vars, (gptr)&p);
 
   return p;
 }
@@ -272,23 +273,23 @@ sp_pcontext::push_cond(LEX_STRING *name, sp_cond_type_t *val)
     p->name.str= name->str;
     p->name.length= name->length;
     p->val= val;
-    insert_dynamic(&m_cond, (gptr)&p);
+    insert_dynamic(&m_conds, (gptr)&p);
   }
 }
 
 /*
- * See comment for find_pvar() above
- */
+  See comment for find_variable() above
+*/
 sp_cond_type_t *
 sp_pcontext::find_cond(LEX_STRING *name, my_bool scoped)
 {
-  uint i= m_cond.elements;
+  uint i= m_conds.elements;
 
   while (i--)
   {
     sp_cond_t *p;
 
-    get_dynamic(&m_cond, (gptr)&p, i);
+    get_dynamic(&m_conds, (gptr)&p, i);
     if (my_strnncoll(system_charset_info,
 		     (const uchar *)name->str, name->length,
 		     (const uchar *)p->name.str, p->name.length) == 0)
@@ -302,20 +303,20 @@ sp_pcontext::find_cond(LEX_STRING *name, my_bool scoped)
 }
 
 /*
- * This only searches the current context, for error checking of
- * duplicates.
- * Returns TRUE if found.
- */
+  This only searches the current context, for error checking of
+  duplicates.
+  Returns TRUE if found.
+*/
 bool
 sp_pcontext::find_handler(sp_cond_type_t *cond)
 {
-  uint i= m_handler.elements;
+  uint i= m_handlers.elements;
 
   while (i--)
   {
     sp_cond_type_t *p;
 
-    get_dynamic(&m_handler, (gptr)&p, i);
+    get_dynamic(&m_handlers, (gptr)&p, i);
     if (cond->type == p->type)
     {
       switch (p->type)
@@ -341,31 +342,31 @@ sp_pcontext::push_cursor(LEX_STRING *name)
 {
   LEX_STRING n;
 
-  if (m_cursor.elements == m_csubsize)
-    m_csubsize+= 1;
+  if (m_cursors.elements == m_max_cursor_index)
+    m_max_cursor_index+= 1;
   n.str= name->str;
   n.length= name->length;
-  insert_dynamic(&m_cursor, (gptr)&n);
+  insert_dynamic(&m_cursors, (gptr)&n);
 }
 
 /*
- * See comment for find_pvar() above
- */
+  See comment for find_variable() above
+*/
 my_bool
 sp_pcontext::find_cursor(LEX_STRING *name, uint *poff, my_bool scoped)
 {
-  uint i= m_cursor.elements;
+  uint i= m_cursors.elements;
 
   while (i--)
   {
     LEX_STRING n;
 
-    get_dynamic(&m_cursor, (gptr)&n, i);
+    get_dynamic(&m_cursors, (gptr)&n, i);
     if (my_strnncoll(system_charset_info,
 		     (const uchar *)name->str, name->length,
 		     (const uchar *)n.str, n.length) == 0)
     {
-      *poff= m_coffset + i;
+      *poff= m_cursor_offset + i;
       return TRUE;
     }
   }
@@ -380,10 +381,10 @@ sp_pcontext::retrieve_field_definitions(List<create_field> *field_def_lst)
 {
   /* Put local/context fields in the result list. */
 
-  for (uint i = 0; i < m_pvar.elements; ++i)
+  for (uint i = 0; i < m_vars.elements; ++i)
   {
-    sp_pvar_t *var_def;
-    get_dynamic(&m_pvar, (gptr) &var_def, i);
+    sp_variable_t *var_def;
+    get_dynamic(&m_vars, (gptr) &var_def, i);
 
     field_def_lst->push_back(&var_def->field_def);
   }
@@ -400,17 +401,17 @@ sp_pcontext::retrieve_field_definitions(List<create_field> *field_def_lst)
 /*
   Find a cursor by offset from the top.
   This is only used for debugging.
- */
+*/
 my_bool
 sp_pcontext::find_cursor(uint offset, LEX_STRING *n)
 {
-  if (m_coffset <= offset && offset < m_coffset + m_cursor.elements)
+  if (m_cursor_offset <= offset &&
+      offset < m_cursor_offset + m_cursors.elements)
   {                           // This frame
-    get_dynamic(&m_cursor, (gptr)n, offset - m_coffset);
+    get_dynamic(&m_cursors, (gptr)n, offset - m_cursor_offset);
     return TRUE;
   }
   if (m_parent)
     return m_parent->find_cursor(offset, n); // Some previous frame
   return FALSE;                 // index out of bounds
 }
-
diff --git a/sql/sp_pcontext.h b/sql/sp_pcontext.h
index 872c7c1d505..e61057537da 100644
--- a/sql/sp_pcontext.h
+++ b/sql/sp_pcontext.h
@@ -29,22 +29,23 @@ typedef enum
   sp_param_inout
 } sp_param_mode_t;
 
-typedef struct sp_pvar
+typedef struct sp_variable
 {
   LEX_STRING name;
   enum enum_field_types type;
   sp_param_mode_t mode;
   
   /*
-    offset -- basically, this is an index of variable in the scope of root
-    parsing context. This means, that all variables in a stored routine
-    have distinct indexes/offsets.
+    offset -- this the index to the variable's value in the runtime frame.
+    This is calculated during parsing and used when creating sp_instr_set
+    instructions and Item_splocal items.
+    I.e. values are set/referred by array indexing in runtime.
   */
   uint offset;
 
   Item *dflt;
   create_field field_def;
-} sp_pvar_t;
+} sp_variable_t;
 
 
 #define SP_LAB_REF   0		// Unresolved reference (for goto)
@@ -76,9 +77,10 @@ typedef struct sp_cond_type
   uint mysqlerr;
 } sp_cond_type_t;
 
-/* Sanity check for SQLSTATEs. Will not check if it's really an existing
- * state (there are just too many), but will check length bad characters.
- */
+/*
+  Sanity check for SQLSTATEs. Will not check if it's really an existing
+  state (there are just too many), but will check length bad characters.
+*/
 extern bool
 sp_cond_check(LEX_STRING *sqlstate);
 
@@ -90,7 +92,17 @@ typedef struct sp_cond
 
 
 /*
-  This seems to be an "SP parsing context" or something.
+  The parse-time context, used to keep track on declared variables/parameters,
+  conditions, handlers, cursors and labels, during parsing.
+  sp_contexts are organized as a tree, with one object for each begin-end
+  block, plus a root-context for the parameters.
+  This is used during parsing for looking up defined names (e.g. declared
+  variables and visible labels), for error checking, and to calculate offsets
+  to be used at runtime. (During execution variable values, active handlers
+  and cursors, etc, are referred to by an index in a stack.)
+  The pcontext tree is also kept during execution and is used for error
+  checking (e.g. correct number of parameters), and in the future, used by
+  the debugger.
 */
 
 class sp_pcontext : public Sql_alloc
@@ -134,50 +146,64 @@ class sp_pcontext : public Sql_alloc
   // Parameters and variables
   //
 
+  /*
+    The maximum number of variables used in this and all child contexts
+    In the root, this gives us the number of slots needed for variables
+    during execution.
+  */
   inline uint
-  total_pvars()
+  max_var_index()
   {
-    return m_total_pvars;
+    return m_max_var_index;
   }
 
+  /*
+    The current number of variables used in the parents (from the root),
+    including this context.
+  */
   inline uint
-  current_pvars()
+  current_var_count()
   {
-    return m_poffset + m_pvar.elements;
+    return m_var_offset + m_vars.elements;
   }
 
+  /* The number of variables in this context alone */
   inline uint
-  context_pvars()
+  context_var_count()
   {
-    return m_pvar.elements;
+    return m_vars.elements;
   }
 
+  /* Map index in this pcontext to runtime offset */
   inline uint
-  pvar_context2index(uint i)
+  var_context2runtime(uint i)
   {
-    return m_poffset + i;
+    return m_var_offset + i;
   }
 
+  /* Set type of variable. 'i' is the offset from the top */
   inline void
   set_type(uint i, enum enum_field_types type)
   {
-    sp_pvar_t *p= find_pvar(i);
+    sp_variable_t *p= find_variable(i);
 
     if (p)
       p->type= type;
   }
 
+  /* Set default value of variable. 'i' is the offset from the top */
   inline void
   set_default(uint i, Item *it)
   {
-    sp_pvar_t *p= find_pvar(i);
+    sp_variable_t *p= find_variable(i);
 
     if (p)
       p->dflt= it;
   }
 
-  sp_pvar_t *
-  push_pvar(LEX_STRING *name, enum enum_field_types type, sp_param_mode_t mode);
+  sp_variable_t *
+  push_variable(LEX_STRING *name, enum enum_field_types type,
+                sp_param_mode_t mode);
 
   /*
     Retrieve definitions of fields from the current context and its
@@ -187,12 +213,12 @@ class sp_pcontext : public Sql_alloc
   retrieve_field_definitions(List<create_field> *field_def_lst);
 
   // Find by name
-  sp_pvar_t *
-  find_pvar(LEX_STRING *name, my_bool scoped=0);
+  sp_variable_t *
+  find_variable(LEX_STRING *name, my_bool scoped=0);
 
-  // Find by offset
-  sp_pvar_t *
-  find_pvar(uint offset);
+  // Find by offset (from the top)
+  sp_variable_t *
+  find_variable(uint offset);
 
   /*
     Set the current scope boundary (for default values).
@@ -280,7 +306,7 @@ class sp_pcontext : public Sql_alloc
   pop_cond(uint num)
   {
     while (num--)
-      pop_dynamic(&m_cond);
+      pop_dynamic(&m_conds);
   }
 
   sp_cond_type_t *
@@ -293,22 +319,22 @@ class sp_pcontext : public Sql_alloc
   inline void
   push_handler(sp_cond_type_t *cond)
   {
-    insert_dynamic(&m_handler, (gptr)&cond);
+    insert_dynamic(&m_handlers, (gptr)&cond);
   }
 
   bool
   find_handler(sp_cond_type *cond);
 
   inline uint
-  max_handlers()
+  max_handler_index()
   {
-    return m_hsubsize + m_handlers;
+    return m_max_handler_index + m_context_handlers;
   }
 
   inline void
   add_handlers(uint n)
   {
-    m_handlers+= n;
+    m_context_handlers+= n;
   }
 
   //
@@ -326,51 +352,51 @@ class sp_pcontext : public Sql_alloc
   find_cursor(uint offset, LEX_STRING *n);
 
   inline uint
-  max_cursors()
+  max_cursor_index()
   {
-    return m_csubsize + m_cursor.elements;
+    return m_max_cursor_index + m_cursors.elements;
   }
 
   inline uint
-  current_cursors()
+  current_cursor_count()
   {
-    return m_coffset + m_cursor.elements;
+    return m_cursor_offset + m_cursors.elements;
   }
 
 protected:
 
   /*
-    m_total_pvars -- number of variables (including all types of arguments)
+    m_max_var_index -- number of variables (including all types of arguments)
     in this context including all children contexts.
     
-    m_total_pvars >= m_pvar.elements.
+    m_max_var_index >= m_vars.elements.
 
-    m_total_pvars of the root parsing context contains number of all
+    m_max_var_index of the root parsing context contains number of all
     variables (including arguments) in all enclosed contexts.
   */
-  uint m_total_pvars;		
+  uint m_max_var_index;		
 
   // The maximum sub context's framesizes
-  uint m_csubsize;
-  uint m_hsubsize;
-  uint m_handlers;		// No. of handlers in this context
+  uint m_max_cursor_index;
+  uint m_max_handler_index;
+  uint m_context_handlers;      // No. of handlers in this context
 
 private:
 
   sp_pcontext *m_parent;	// Parent context
 
   /*
-    m_poffset -- basically, this is an index of the first variable in this
-    parsing context.
+    m_var_offset -- this is an index of the first variable in this
+                    parsing context.
     
-    m_poffset is 0 for root context.
+    m_var_offset is 0 for root context.
 
     Since now each variable is stored in separate place, no reuse is done,
-    so m_poffset is different for all enclosed contexts.
+    so m_var_offset is different for all enclosed contexts.
   */
-  uint m_poffset;
+  uint m_var_offset;
 
-  uint m_coffset;		// Cursor offset for this context
+  uint m_cursor_offset;		// Cursor offset for this context
 
   /*
     Boundary for finding variables in this context. This is the number
@@ -382,11 +408,11 @@ private:
 
   int m_num_case_exprs;
 
-  DYNAMIC_ARRAY m_pvar;		// Parameters/variables
+  DYNAMIC_ARRAY m_vars;		// Parameters/variables
   DYNAMIC_ARRAY m_case_expr_id_lst; /* Stack of CASE expression ids. */
-  DYNAMIC_ARRAY m_cond;		// Conditions
-  DYNAMIC_ARRAY m_cursor;	// Cursors
-  DYNAMIC_ARRAY m_handler;	// Handlers, for checking of duplicates
+  DYNAMIC_ARRAY m_conds;        // Conditions
+  DYNAMIC_ARRAY m_cursors;	// Cursors
+  DYNAMIC_ARRAY m_handlers;	// Handlers, for checking for duplicates
 
   List<sp_label_t> m_label;	// The label list
 
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index af4e41c29be..38b6de0e75a 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -73,16 +73,16 @@ bool sp_rcontext::init(THD *thd)
 
   return
     !(m_handler=
-      (sp_handler_t*)thd->alloc(m_root_parsing_ctx->max_handlers() *
+      (sp_handler_t*)thd->alloc(m_root_parsing_ctx->max_handler_index() *
                                 sizeof(sp_handler_t))) ||
     !(m_hstack=
-      (uint*)thd->alloc(m_root_parsing_ctx->max_handlers() *
+      (uint*)thd->alloc(m_root_parsing_ctx->max_handler_index() *
                         sizeof(uint))) ||
     !(m_in_handler=
-      (uint*)thd->alloc(m_root_parsing_ctx->max_handlers() *
+      (uint*)thd->alloc(m_root_parsing_ctx->max_handler_index() *
                         sizeof(uint))) ||
     !(m_cstack=
-      (sp_cursor**)thd->alloc(m_root_parsing_ctx->max_cursors() *
+      (sp_cursor**)thd->alloc(m_root_parsing_ctx->max_cursor_index() *
                               sizeof(sp_cursor*))) ||
     !(m_case_expr_holders=
       (Item_cache**)thd->calloc(m_root_parsing_ctx->get_num_case_exprs() *
@@ -105,12 +105,12 @@ sp_rcontext::init_var_table(THD *thd)
 {
   List<create_field> field_def_lst;
 
-  if (!m_root_parsing_ctx->total_pvars())
+  if (!m_root_parsing_ctx->max_var_index())
     return FALSE;
 
   m_root_parsing_ctx->retrieve_field_definitions(&field_def_lst);
 
-  DBUG_ASSERT(field_def_lst.elements == m_root_parsing_ctx->total_pvars());
+  DBUG_ASSERT(field_def_lst.elements == m_root_parsing_ctx->max_var_index());
   
   if (!(m_var_table= create_virtual_tmp_table(thd, field_def_lst)))
     return TRUE;
@@ -134,7 +134,7 @@ bool
 sp_rcontext::init_var_items()
 {
   uint idx;
-  uint num_vars= m_root_parsing_ctx->total_pvars();
+  uint num_vars= m_root_parsing_ctx->max_var_index();
 
   if (!(m_var_items= (Item**) sql_alloc(num_vars * sizeof (Item *))))
     return TRUE;
@@ -381,7 +381,7 @@ sp_cursor::destroy()
 
 
 int
-sp_cursor::fetch(THD *thd, List<struct sp_pvar> *vars)
+sp_cursor::fetch(THD *thd, List<struct sp_variable> *vars)
 {
   if (! server_side_cursor)
   {
@@ -528,9 +528,9 @@ int Select_fetch_into_spvars::prepare(List<Item> &fields, SELECT_LEX_UNIT *u)
 
 bool Select_fetch_into_spvars::send_data(List<Item> &items)
 {
-  List_iterator_fast<struct sp_pvar> pv_iter(*spvar_list);
+  List_iterator_fast<struct sp_variable> spvar_iter(*spvar_list);
   List_iterator_fast<Item> item_iter(items);
-  sp_pvar_t *pv;
+  sp_variable_t *spvar;
   Item *item;
 
   /* Must be ensured by the caller */
@@ -540,9 +540,9 @@ bool Select_fetch_into_spvars::send_data(List<Item> &items)
     Assign the row fetched from a server side cursor to stored
     procedure variables.
   */
-  for (; pv= pv_iter++, item= item_iter++; )
+  for (; spvar= spvar_iter++, item= item_iter++; )
   {
-    if (thd->spcont->set_variable(thd, pv->offset, item))
+    if (thd->spcont->set_variable(thd, spvar->offset, item))
       return TRUE;
   }
   return FALSE;
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index e7393902e72..20aaea3b7c1 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -24,7 +24,7 @@
 
 struct sp_cond_type;
 class sp_cursor;
-struct sp_pvar;
+struct sp_variable;
 class sp_lex_keeper;
 class sp_instr_cpush;
 
@@ -265,12 +265,12 @@ private:
 
 class Select_fetch_into_spvars: public select_result_interceptor
 {
-  List<struct sp_pvar> *spvar_list;
+  List<struct sp_variable> *spvar_list;
   uint field_count;
 public:
   Select_fetch_into_spvars() {}               /* Remove gcc warning */
   uint get_field_count() { return field_count; }
-  void set_spvar_list(List<struct sp_pvar> *vars) { spvar_list= vars; }
+  void set_spvar_list(List<struct sp_variable> *vars) { spvar_list= vars; }
 
   virtual bool send_eof() { return FALSE; }
   virtual bool send_data(List<Item> &items);
@@ -307,7 +307,7 @@ public:
   }
 
   int
-  fetch(THD *, List<struct sp_pvar> *vars);
+  fetch(THD *, List<struct sp_variable> *vars);
 
   inline sp_instr_cpush *
   get_instr()
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
index ddc267eb970..37f72c351e5 100644
--- a/sql/sql_yacc.yy
+++ b/sql/sql_yacc.yy
@@ -1461,22 +1461,23 @@ sp_fdparam:
 	    LEX *lex= Lex;
 	    sp_pcontext *spc= lex->spcont;
 
-	    if (spc->find_pvar(&$1, TRUE))
+	    if (spc->find_variable(&$1, TRUE))
 	    {
 	      my_error(ER_SP_DUP_PARAM, MYF(0), $1.str);
 	      YYABORT;
 	    }
-	    sp_pvar_t *pvar= spc->push_pvar(&$1, (enum enum_field_types)$3,
-                                            sp_param_in);
+            sp_variable_t *spvar= spc->push_variable(&$1,
+                                                     (enum enum_field_types)$3,
+                                                     sp_param_in);
 
             if (lex->sphead->fill_field_definition(YYTHD, lex,
                                                    (enum enum_field_types) $3,
-                                                   &pvar->field_def))
+                                                   &spvar->field_def))
             {
               YYABORT;
             }
-            pvar->field_def.field_name= pvar->name.str;
-            pvar->field_def.pack_flag |= FIELDFLAG_MAYBE_NULL;
+            spvar->field_def.field_name= spvar->name.str;
+            spvar->field_def.pack_flag |= FIELDFLAG_MAYBE_NULL;
 	  }
 	;
 
@@ -1497,22 +1498,23 @@ sp_pdparam:
 	    LEX *lex= Lex;
 	    sp_pcontext *spc= lex->spcont;
 
-	    if (spc->find_pvar(&$3, TRUE))
+	    if (spc->find_variable(&$3, TRUE))
 	    {
 	      my_error(ER_SP_DUP_PARAM, MYF(0), $3.str);
 	      YYABORT;
 	    }
-	    sp_pvar_t *pvar= spc->push_pvar(&$3, (enum enum_field_types)$4,
-			                    (sp_param_mode_t)$1);
+            sp_variable_t *spvar= spc->push_variable(&$3,
+                                                     (enum enum_field_types)$4,
+                                                     (sp_param_mode_t)$1);
 
             if (lex->sphead->fill_field_definition(YYTHD, lex,
                                                    (enum enum_field_types) $4,
-                                                   &pvar->field_def))
+                                                   &spvar->field_def))
             {
               YYABORT;
             }
-            pvar->field_def.field_name= pvar->name.str;
-            pvar->field_def.pack_flag |= FIELDFLAG_MAYBE_NULL;
+            spvar->field_def.field_name= spvar->name.str;
+            spvar->field_def.pack_flag |= FIELDFLAG_MAYBE_NULL;
 	  }
 	;
 
@@ -1576,7 +1578,7 @@ sp_decl:
           {
             LEX *lex= Lex;
             sp_pcontext *pctx= lex->spcont;
-            uint num_vars= pctx->context_pvars();
+            uint num_vars= pctx->context_var_count();
             enum enum_field_types var_type= (enum enum_field_types) $4;
             Item *dflt_value_item= $5;
             create_field *create_field_op;
@@ -1589,23 +1591,23 @@ sp_decl:
             
             for (uint i = num_vars-$2 ; i < num_vars ; i++)
             {
-              uint var_idx= pctx->pvar_context2index(i);
-              sp_pvar_t *pvar= pctx->find_pvar(var_idx);
+              uint var_idx= pctx->var_context2runtime(i);
+              sp_variable_t *spvar= pctx->find_variable(var_idx);
             
-              if (!pvar)
+              if (!spvar)
                 YYABORT;
             
-              pvar->type= var_type;
-              pvar->dflt= dflt_value_item;
+              spvar->type= var_type;
+              spvar->dflt= dflt_value_item;
             
               if (lex->sphead->fill_field_definition(YYTHD, lex, var_type,
-                                                     &pvar->field_def))
+                                                     &spvar->field_def))
               {
                 YYABORT;
               }
             
-              pvar->field_def.field_name= pvar->name.str;
-              pvar->field_def.pack_flag |= FIELDFLAG_MAYBE_NULL;
+              spvar->field_def.field_name= spvar->name.str;
+              spvar->field_def.pack_flag |= FIELDFLAG_MAYBE_NULL;
             
               /* The last instruction is responsible for freeing LEX. */
 
@@ -1642,7 +1644,7 @@ sp_decl:
 	    sp_pcontext *ctx= lex->spcont;
 	    sp_instr_hpush_jump *i=
               new sp_instr_hpush_jump(sp->instructions(), ctx, $2,
-	                              ctx->current_pvars());
+	                              ctx->current_var_count());
 
 	    sp->add_instr(i);
 	    sp->push_backpatch(i, ctx->push_label((char *)"", 0));
@@ -1659,7 +1661,7 @@ sp_decl:
 	    if ($2 == SP_HANDLER_CONTINUE)
 	    {
 	      i= new sp_instr_hreturn(sp->instructions(), ctx,
-	                              ctx->current_pvars());
+	                              ctx->current_var_count());
 	      sp->add_instr(i);
 	    }
 	    else
@@ -1690,7 +1692,7 @@ sp_decl:
 	      YYABORT;
 	    }
             i= new sp_instr_cpush(sp->instructions(), ctx, $5,
-                                  ctx->current_cursors());
+                                  ctx->current_cursor_count());
 	    sp->add_instr(i);
 	    ctx->push_cursor(&$2);
 	    $$.vars= $$.conds= $$.hndlrs= 0;
@@ -1845,12 +1847,12 @@ sp_decl_idents:
 	    LEX *lex= Lex;
 	    sp_pcontext *spc= lex->spcont;
 
-	    if (spc->find_pvar(&$1, TRUE))
+	    if (spc->find_variable(&$1, TRUE))
 	    {
 	      my_error(ER_SP_DUP_VAR, MYF(0), $1.str);
 	      YYABORT;
 	    }
-	    spc->push_pvar(&$1, (enum_field_types)0, sp_param_in);
+	    spc->push_variable(&$1, (enum_field_types)0, sp_param_in);
 	    $$= 1;
 	  }
 	| sp_decl_idents ',' ident
@@ -1860,12 +1862,12 @@ sp_decl_idents:
 	    LEX *lex= Lex;
 	    sp_pcontext *spc= lex->spcont;
 
-	    if (spc->find_pvar(&$3, TRUE))
+	    if (spc->find_variable(&$3, TRUE))
 	    {
 	      my_error(ER_SP_DUP_VAR, MYF(0), $3.str);
 	      YYABORT;
 	    }
-	    spc->push_pvar(&$3, (enum_field_types)0, sp_param_in);
+	    spc->push_variable(&$3, (enum_field_types)0, sp_param_in);
 	    $$= $1 + 1;
 	  }
 	;
@@ -2198,9 +2200,9 @@ sp_fetch_list:
 	    LEX *lex= Lex;
 	    sp_head *sp= lex->sphead;
 	    sp_pcontext *spc= lex->spcont;
-	    sp_pvar_t *spv;
+	    sp_variable_t *spv;
 
-	    if (!spc || !(spv = spc->find_pvar(&$1)))
+	    if (!spc || !(spv = spc->find_variable(&$1)))
 	    {
 	      my_error(ER_SP_UNDECLARED_VAR, MYF(0), $1.str);
 	      YYABORT;
@@ -2219,9 +2221,9 @@ sp_fetch_list:
 	    LEX *lex= Lex;
 	    sp_head *sp= lex->sphead;
 	    sp_pcontext *spc= lex->spcont;
-	    sp_pvar_t *spv;
+	    sp_variable_t *spv;
 
-	    if (!spc || !(spv = spc->find_pvar(&$3)))
+	    if (!spc || !(spv = spc->find_variable(&$3)))
 	    {
 	      my_error(ER_SP_UNDECLARED_VAR, MYF(0), $3.str);
 	      YYABORT;
@@ -5868,9 +5870,9 @@ select_var_ident:
            | ident_or_text
            {
              LEX *lex=Lex;
-	     sp_pvar_t *t;
+	     sp_variable_t *t;
 
-	     if (!lex->spcont || !(t=lex->spcont->find_pvar(&$1)))
+	     if (!lex->spcont || !(t=lex->spcont->find_variable(&$1)))
 	     {
 	       my_error(ER_SP_UNDECLARED_VAR, MYF(0), $1.str);
 	       YYABORT;
@@ -7200,10 +7202,10 @@ order_ident:
 simple_ident:
 	ident
 	{
-	  sp_pvar_t *spv;
+	  sp_variable_t *spv;
 	  LEX *lex = Lex;
           sp_pcontext *spc = lex->spcont;
-	  if (spc && (spv = spc->find_pvar(&$1)))
+	  if (spc && (spv = spc->find_variable(&$1)))
 	  {
             /* We're compiling a stored procedure and found a variable */
             Item_splocal *splocal;
@@ -7947,7 +7949,7 @@ sys_option_value:
           {
             /* An SP local variable */
             sp_pcontext *ctx= lex->spcont;
-            sp_pvar_t *spv;
+            sp_variable_t *spv;
             sp_instr_set *sp_set;
             Item *it;
             if ($1)
@@ -7956,7 +7958,7 @@ sys_option_value:
               YYABORT;
             }
 
-            spv= ctx->find_pvar(&$2.base_name);
+            spv= ctx->find_variable(&$2.base_name);
 
             if ($4)
               it= $4;
@@ -8006,7 +8008,7 @@ option_value:
 
 	    names.str= (char *)"names";
 	    names.length= 5;
-	    if (spc && spc->find_pvar(&names))
+	    if (spc && spc->find_variable(&names))
               my_error(ER_SP_BAD_VAR_SHADOW, MYF(0), names.str);
             else
               yyerror(ER(ER_SYNTAX_ERROR));
@@ -8036,7 +8038,7 @@ option_value:
 
 	    pw.str= (char *)"password";
 	    pw.length= 8;
-	    if (spc && spc->find_pvar(&pw))
+	    if (spc && spc->find_variable(&pw))
 	    {
               my_error(ER_SP_BAD_VAR_SHADOW, MYF(0), pw.str);
 	      YYABORT;
@@ -8058,10 +8060,10 @@ internal_variable_name:
 	{
 	  LEX *lex= Lex;
           sp_pcontext *spc= lex->spcont;
-	  sp_pvar_t *spv;
+	  sp_variable_t *spv;
 
 	  /* We have to lookup here since local vars can shadow sysvars */
-	  if (!spc || !(spv = spc->find_pvar(&$1)))
+	  if (!spc || !(spv = spc->find_variable(&$1)))
 	  {
             /* Not an SP local variable */
 	    sys_var *tmp=find_sys_var($1.str, $1.length);
-- 
cgit v1.2.1


From 51a3d3668fdbfd25588dd92038f6d82dc7eb8306 Mon Sep 17 00:00:00 2001
From: unknown <kroki@mysql.com>
Date: Wed, 12 Apr 2006 17:37:57 +0400
Subject: In test for bug#15933 we have to wait for all disconnects to finish
 to avoid a race between updating and checking Max_used_connections.  This is
 done in a loop until either disconnect finished or timeout expired.  In a
 latter case the test will fail.

mysql-test/r/status.result:
  Update result to match changes in test case.
mysql-test/t/status.test:
  Close extra conections in previous test.
  In test for bug#15933 we have to wait for all disconnects to finish to avoid
  a race between updating and checking Max_used_connections.  This is done in
  a loop until either disconnect finished or timeout expired.  In a latter case
  the test will fail.
  Use con1, con2, con3 instead of con3, con4, con5.
---
 mysql-test/r/status.result | 10 +++----
 mysql-test/t/status.test   | 65 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/mysql-test/r/status.result b/mysql-test/r/status.result
index 4e5489191c9..ca21b333a6a 100644
--- a/mysql-test/r/status.result
+++ b/mysql-test/r/status.result
@@ -26,20 +26,20 @@ Last_query_cost	0.000000
 FLUSH STATUS;
 SHOW STATUS LIKE 'max_used_connections';
 Variable_name	Value
-Max_used_connections	3
+Max_used_connections	1
 SET @save_thread_cache_size=@@thread_cache_size;
 SET GLOBAL thread_cache_size=3;
 SHOW STATUS LIKE 'max_used_connections';
 Variable_name	Value
-Max_used_connections	5
+Max_used_connections	3
 FLUSH STATUS;
 SHOW STATUS LIKE 'max_used_connections';
 Variable_name	Value
-Max_used_connections	4
+Max_used_connections	2
 SHOW STATUS LIKE 'max_used_connections';
 Variable_name	Value
-Max_used_connections	5
+Max_used_connections	3
 SHOW STATUS LIKE 'max_used_connections';
 Variable_name	Value
-Max_used_connections	6
+Max_used_connections	4
 SET GLOBAL thread_cache_size=@save_thread_cache_size;
diff --git a/mysql-test/t/status.test b/mysql-test/t/status.test
index 79a46a38550..1a71425d2a7 100644
--- a/mysql-test/t/status.test
+++ b/mysql-test/t/status.test
@@ -36,6 +36,10 @@ reap;
 show status like 'Table_lock%';
 drop table t1;
 
+disconnect con2;
+disconnect con1;
+connection default;
+
 # End of 4.1 tests
 
 #
@@ -60,48 +64,81 @@ show status like 'last_query_cost';
 #   taking a thread from the cache as well as when creating new threads
 #
 
-# Previous test uses con1, con2.  If we disconnect them, there will be
-# a race on when exactly this will happen, so we better leave them as
-# is.
-connection default;
+# Wait for at most $disconnect_timeout seconds for disconnects to finish.
+let $disconnect_timeout = 10;
 
-# Reset max_used_connections from previous tests.
+# Wait for any previous disconnects to finish.
 FLUSH STATUS;
+--disable_query_log
+--disable_result_log
+eval SET @wait_left = $disconnect_timeout;
+let $max_used_connections = `SHOW STATUS LIKE 'max_used_connections'`;
+eval SET @max_used_connections = SUBSTRING('$max_used_connections', 21)+0;
+let $wait_more = `SELECT @max_used_connections != 1 && @wait_left > 0`;
+while ($wait_more)
+{
+  sleep 1;
+  FLUSH STATUS;
+  SET @wait_left = @wait_left - 1;
+  let $max_used_connections = `SHOW STATUS LIKE 'max_used_connections'`;
+  eval SET @max_used_connections = SUBSTRING('$max_used_connections', 21)+0;
+  let $wait_more = `SELECT @max_used_connections != 1 && @wait_left > 0`;
+}
+--enable_query_log
+--enable_result_log
+
+# Prerequisite.
 SHOW STATUS LIKE 'max_used_connections';
 
 # Save original setting.
 SET @save_thread_cache_size=@@thread_cache_size;
 SET GLOBAL thread_cache_size=3;
 
-connect (con3,localhost,root,,);
-connect (con4,localhost,root,,);
+connect (con1,localhost,root,,);
+connect (con2,localhost,root,,);
 
-connection con3;
-disconnect con4;
+connection con1;
+disconnect con2;
 
 # Check that max_used_connections still reflects maximum value.
 SHOW STATUS LIKE 'max_used_connections';
 
 # Check that after flush max_used_connections equals to current number
-# of connections.
+# of connections.  First wait for previous disconnect to finish.
 FLUSH STATUS;
+--disable_query_log
+--disable_result_log
+eval SET @wait_left = $disconnect_timeout;
+let $max_used_connections = `SHOW STATUS LIKE 'max_used_connections'`;
+eval SET @max_used_connections = SUBSTRING('$max_used_connections', 21)+0;
+let $wait_more = `SELECT @max_used_connections != 2 && @wait_left > 0`;
+while ($wait_more)
+{
+  sleep 1;
+  FLUSH STATUS;
+  SET @wait_left = @wait_left - 1;
+  let $max_used_connections = `SHOW STATUS LIKE 'max_used_connections'`;
+  eval SET @max_used_connections = SUBSTRING('$max_used_connections', 21)+0;
+  let $wait_more = `SELECT @max_used_connections != 2 && @wait_left > 0`;
+}
+--enable_query_log
+--enable_result_log
+# Check that we don't count disconnected thread any longer.
 SHOW STATUS LIKE 'max_used_connections';
 
 # Check that max_used_connections is updated when cached thread is
 # reused...
-connect (con4,localhost,root,,);
+connect (con2,localhost,root,,);
 SHOW STATUS LIKE 'max_used_connections';
 
 # ...and when new thread is created.
-connect (con5,localhost,root,,);
+connect (con3,localhost,root,,);
 SHOW STATUS LIKE 'max_used_connections';
 
 # Restore original setting.
 connection default;
 SET GLOBAL thread_cache_size=@save_thread_cache_size;
 
-disconnect con5;
-disconnect con4;
 disconnect con3;
 disconnect con2;
 disconnect con1;
-- 
cgit v1.2.1


From 886a35bd82c153253007040202116b5f634705f4 Mon Sep 17 00:00:00 2001
From: unknown <kroki@mysql.com>
Date: Wed, 12 Apr 2006 19:31:00 +0400
Subject: Bug#16461: connection_id() does not work properly inside trigger

CONNECTION_ID() was implemented as a constant Item, i.e. an instance of
Item_static_int_func class holding value computed at creation time.
Since Items are created on parsing, and trigger statements are parsed
on table open, the first connection to open a particular table would
effectively set its own CONNECTION_ID() inside trigger statements for
that table.

Re-implement CONNECTION_ID() as a class derived from Item_int_func, and
compute connection_id on every call to fix_fields().


mysql-test/r/trigger.result:
  Add result for bug#16461.
mysql-test/t/trigger.test:
  Add test case for bug#16461.
sql/item.cc:
  Remove now unused class Item_static_int_func.
sql/item.h:
  Remove now unused class Item_static_int_func.
sql/item_create.cc:
  Use new implementation of CONNECTION_ID().
sql/item_func.cc:
  Re-implement CONNECTION_ID() as Item_func_connection_id
  (was Item_static_int_func).  Set max_length to 10, as it was before.
  Compute connection_id dynamically on every call to fix_fields().
sql/item_func.h:
  Re-implement CONNECTION_ID() as Item_func_connection_id
  (was Item_static_int_func).
---
 mysql-test/r/trigger.result | 13 +++++++++++++
 mysql-test/t/trigger.test   | 28 ++++++++++++++++++++++++++++
 sql/item.cc                 | 16 ----------------
 sql/item.h                  | 12 ------------
 sql/item_create.cc          | 10 ++--------
 sql/item_func.cc            | 25 +++++++++++++++++++++++++
 sql/item_func.h             | 12 ++++++++++++
 7 files changed, 80 insertions(+), 36 deletions(-)

diff --git a/mysql-test/r/trigger.result b/mysql-test/r/trigger.result
index 828db549b05..681b805f547 100644
--- a/mysql-test/r/trigger.result
+++ b/mysql-test/r/trigger.result
@@ -952,3 +952,16 @@ load data infile '../std_data_ln/words.dat' into table t1 (a) set b:= f1();
 drop table t1;
 drop function f1;
 drop function f2;
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (
+conn_id INT,
+trigger_conn_id INT
+);
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+SET NEW.trigger_conn_id = CONNECTION_ID();
+INSERT INTO t1 (conn_id, trigger_conn_id) VALUES (CONNECTION_ID(), -1);
+INSERT INTO t1 (conn_id, trigger_conn_id) VALUES (CONNECTION_ID(), -1);
+SELECT * FROM t1 WHERE conn_id != trigger_conn_id;
+conn_id	trigger_conn_id
+DROP TRIGGER t1_bi;
+DROP TABLE t1;
diff --git a/mysql-test/t/trigger.test b/mysql-test/t/trigger.test
index c5925bbd9d5..a0b67b2204d 100644
--- a/mysql-test/t/trigger.test
+++ b/mysql-test/t/trigger.test
@@ -1114,3 +1114,31 @@ load data infile '../std_data_ln/words.dat' into table t1 (a) set b:= f1();
 drop table t1;
 drop function f1;
 drop function f2;
+
+#
+# Test for Bug #16461 connection_id() does not work properly inside trigger
+#
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (
+    conn_id INT,
+    trigger_conn_id INT
+);
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+  SET NEW.trigger_conn_id = CONNECTION_ID();
+
+INSERT INTO t1 (conn_id, trigger_conn_id) VALUES (CONNECTION_ID(), -1);
+
+connect (con1,localhost,root,,);
+INSERT INTO t1 (conn_id, trigger_conn_id) VALUES (CONNECTION_ID(), -1);
+connection default;
+disconnect con1;
+
+SELECT * FROM t1 WHERE conn_id != trigger_conn_id;
+
+DROP TRIGGER t1_bi;
+DROP TABLE t1;
+
+# End of 5.0 tests
diff --git a/sql/item.cc b/sql/item.cc
index 45a6b0cef31..4d3ce6071eb 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -644,22 +644,6 @@ Item *Item_num::safe_charset_converter(CHARSET_INFO *tocs)
 }
 
 
-Item *Item_static_int_func::safe_charset_converter(CHARSET_INFO *tocs)
-{
-  Item_string *conv;
-  char buf[64];
-  String *s, tmp(buf, sizeof(buf), &my_charset_bin);
-  s= val_str(&tmp);
-  if ((conv= new Item_static_string_func(func_name, s->ptr(), s->length(),
-                                         s->charset())))
-  {
-    conv->str_value.copy();
-    conv->str_value.mark_as_const();
-  }
-  return conv;
-}
-
-
 Item *Item_static_float_func::safe_charset_converter(CHARSET_INFO *tocs)
 {
   Item_string *conv;
diff --git a/sql/item.h b/sql/item.h
index 3b96091af38..d33e0ae34be 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -1387,18 +1387,6 @@ public:
 };
 
 
-class Item_static_int_func :public Item_int
-{
-  const char *func_name;
-public:
-  Item_static_int_func(const char *str_arg, longlong i, uint length)
-    :Item_int(NullS, i, length), func_name(str_arg)
-  {}
-  Item *safe_charset_converter(CHARSET_INFO *tocs);
-  void print(String *str) { str->append(func_name); }
-};
-
-
 class Item_uint :public Item_int
 {
 public:
diff --git a/sql/item_create.cc b/sql/item_create.cc
index 342ef245a76..bfcb2101d60 100644
--- a/sql/item_create.cc
+++ b/sql/item_create.cc
@@ -71,14 +71,8 @@ Item *create_func_ceiling(Item* a)
 
 Item *create_func_connection_id(void)
 {
-  THD *thd=current_thd;
-  thd->lex->safe_to_cache_query= 0;
-  return new Item_static_int_func("connection_id()",
-                                  (longlong)
-                                  ((thd->slave_thread) ?
-                                   thd->variables.pseudo_thread_id :
-                                   thd->thread_id),
-                                  10);
+  current_thd->lex->safe_to_cache_query= 0;
+  return new Item_func_connection_id();
 }
 
 Item *create_func_conv(Item* a, Item *b, Item *c)
diff --git a/sql/item_func.cc b/sql/item_func.cc
index f40f868f75f..d4a0e607fc2 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -559,6 +559,31 @@ String *Item_int_func::val_str(String *str)
 }
 
 
+void Item_func_connection_id::fix_length_and_dec()
+{
+  Item_int_func::fix_length_and_dec();
+  max_length= 10;
+}
+
+
+bool Item_func_connection_id::fix_fields(THD *thd, Item **ref)
+{
+  if (Item_int_func::fix_fields(thd, ref))
+    return TRUE;
+
+  /*
+    To replicate CONNECTION_ID() properly we should use
+    pseudo_thread_id on slave, which contains the value of thread_id
+    on master.
+  */
+  value= ((thd->slave_thread) ?
+          thd->variables.pseudo_thread_id :
+          thd->thread_id);
+
+  return FALSE;
+}
+
+
 /*
   Check arguments here to determine result's type for a numeric
   function of two arguments.
diff --git a/sql/item_func.h b/sql/item_func.h
index ccbbbab1df4..ed5924e8fe1 100644
--- a/sql/item_func.h
+++ b/sql/item_func.h
@@ -279,6 +279,18 @@ public:
 };
 
 
+class Item_func_connection_id :public Item_int_func
+{
+  longlong value;
+
+public:
+  const char *func_name() const { return "connection_id"; }
+  void fix_length_and_dec();
+  bool fix_fields(THD *thd, Item **ref);
+  longlong val_int() { DBUG_ASSERT(fixed == 1); return value; }
+};
+
+
 class Item_func_signed :public Item_int_func
 {
 public:
-- 
cgit v1.2.1


From 04f93ca02ab3715d6b16d36dcfda7b6d8c8d791c Mon Sep 17 00:00:00 2001
From: unknown <konstantin@mysql.com>
Date: Sun, 23 Apr 2006 04:09:56 +0400
Subject: A post-merge fix.

---
 sql/mysqld.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index ed10286f91e..923eb530099 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -8071,8 +8071,9 @@ void refresh_status(THD *thd)
   add_to_status(&global_status_var, &thd->status_var);
   bzero((char*) &thd->status_var, sizeof(thd->status_var));
 
-  for (struct show_var_st *ptr=status_vars; ptr->name; ptr++)
+  for (SHOW_VAR *ptr= status_vars; ptr->name; ptr++)
   {
+    /* Note that SHOW_LONG_NOFLUSH variables are not reset */
     if (ptr->type == SHOW_LONG)
       *(ulong*) ptr->value= 0;
   }
-- 
cgit v1.2.1