Fixed BUG#9598: stored procedure call within stored procedure

                overwrites IN variable
  and added error checking of variables for [IN]OUT parameters while
  rewriting the out parameter handling.


mysql-test/r/sp-error.result:
  New test case for non-variable argument for [IN]OUT parameters.
  (And changed to qualified names in some other error messages.)
mysql-test/r/sp.result:
  New test case for BUG#9598.
mysql-test/t/sp-error.test:
  New test case for non-variable argument for [IN]OUT parameters.
mysql-test/t/sp.test:
  New test case for BUG#9598.
sql/item.h:
  Need to distinguish between SP local variable items and other items,
  for error checking and [IN]OUT parameter handling.
sql/share/errmsg.txt:
  New error message for non-variable arguments for [IN]OUT parameters in stored procedures.
sql/sp_head.cc:
  Rewrote the [IN]OUT parameter handling in procedure invokation, to make
  it work properly when using user variables in sub-calls.
  Also added error checking for non-variable arguments for such parameters
  (and changed to qualified names for wrong number of arg. errors).
sql/sp_rcontext.cc:
  No need to keep track on the out index for an [IN]OUT parameter any more.
sql/sp_rcontext.h:
  No need to keep track on the out index for an [IN]OUT parameter any more.

5 files changed, 53 insertions, 55 deletions

diff --git a/sql/item.h b/sql/item.h
index f4ca292e574..f7bb98bb8ad 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -545,6 +545,8 @@ public:
     cleanup();
     delete this;
   }
+
+  virtual bool is_splocal() { return 0; } /* Needed for error checking */
 };
 
 
@@ -564,6 +566,8 @@ public:
     Item::maybe_null= TRUE;
   }
 
+  bool is_splocal() { return 1; } /* Needed for error checking */
+
   Item *this_item();
   Item *this_const_item() const;
 
diff --git a/sql/share/errmsg.txt b/sql/share/errmsg.txt
index 95fb1736741..e93229a4a3e 100644
--- a/sql/share/errmsg.txt
+++ b/sql/share/errmsg.txt
@@ -5340,3 +5340,5 @@ ER_TABLE_DEF_CHANGED
 	eng "Table definition has changed, please retry transaction"
 ER_SP_DUP_HANDLER 42000
 	eng "Duplicate handler declared in the same block"
+ER_SP_NOT_VAR_ARG 42000
+	eng "OUT or INOUT argument %d for routine %s is not a variable"
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 63f67959f33..0fe9c449540 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -638,7 +638,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
     // Need to use my_printf_error here, or it will not terminate the
     // invoking query properly.
     my_error(ER_SP_WRONG_NO_OF_ARGS, MYF(0),
-             "FUNCTION", m_name.str, params, argcount);
+             "FUNCTION", m_qname.str, params, argcount);
     DBUG_RETURN(-1);
   }
 
@@ -691,6 +691,19 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount, Item **resp)
   DBUG_RETURN(ret);
 }
 
+static Item_func_get_user_var *
+item_is_user_var(Item *it)
+{
+  if (it->type() == Item::FUNC_ITEM)
+  {
+    Item_func *fi= static_cast<Item_func*>(it);
+
+    if (fi->functype() == Item_func::GUSERVAR_FUNC)
+      return static_cast<Item_func_get_user_var*>(fi);
+  }
+  return NULL;
+}
+
 int
 sp_head::execute_procedure(THD *thd, List<Item> *args)
 {
@@ -708,7 +721,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
   if (args->elements != params)
   {
     my_error(ER_SP_WRONG_NO_OF_ARGS, MYF(0), "PROCEDURE",
-             m_name.str, params, args->elements);
+             m_qname.str, params, args->elements);
     DBUG_RETURN(-1);
   }
 
@@ -728,12 +741,19 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
     // QQ: Should do type checking?
     for (i = 0 ; (it= li++) && i < params ; i++)
     {
-      sp_pvar_t *pvar = m_pcont->find_pvar(i);
+      sp_pvar_t *pvar= m_pcont->find_pvar(i);
 
-      if (! pvar)
-	nctx->set_oindex(i, -1); // Shouldn't happen
-      else
+      if (pvar)
       {
+	if (pvar->mode != sp_param_in)
+	{
+	  if (!it->is_splocal() && !item_is_user_var(it))
+	  {
+	    my_error(ER_SP_NOT_VAR_ARG, MYF(0), i+1, m_qname.str);
+	    ret= -1;
+	    break;
+	  }
+	}
 	if (pvar->mode == sp_param_out)
 	{
 	  if (! nit)
@@ -742,7 +762,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 	}
 	else
 	{
-	  Item *it2= sp_eval_func_item(thd, it,pvar->type);
+	  Item *it2= sp_eval_func_item(thd, it, pvar->type);
 
 	  if (it2)
 	    nctx->push_item(it2); // IN or INOUT
@@ -752,13 +772,6 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 	    break;
 	  }
 	}
-	// Note: If it's OUT or INOUT, it must be a variable.
-	// QQ: We can check for global variables here, or should we do it
-	//     while parsing?
-	if (pvar->mode == sp_param_in)
-	  nctx->set_oindex(i, -1); // IN
-	else			// OUT or INOUT
-	  nctx->set_oindex(i, static_cast<Item_splocal *>(it)->get_offset());
       }
     }
 
@@ -786,38 +799,31 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
     // set global user variables
     for (uint i = 0 ; (it= li++) && i < params ; i++)
     {
-      int oi = nctx->get_oindex(i);
+      sp_pvar_t *pvar= m_pcont->find_pvar(i);
 
-      if (oi >= 0)
+      if (pvar->mode != sp_param_in)
       {
-	if (! tmp_octx)
-	  octx->set_item(nctx->get_oindex(i), nctx->get_item(i));
+	if (it->is_splocal())
+	  octx->set_item(static_cast<Item_splocal *>(it)->get_offset(),
+			 nctx->get_item(i));
 	else
 	{
-	  // QQ Currently we just silently ignore non-user-variable arguments.
-	  //    We should check this during parsing, when setting up the call
-	  //    above
-	  if (it->type() == Item::FUNC_ITEM)
+	  Item_func_get_user_var *guv= item_is_user_var(it);
+
+	  if (guv)
 	  {
-	    Item_func *fi= static_cast<Item_func*>(it);
-
-	    if (fi->functype() == Item_func::GUSERVAR_FUNC)
-	    {			// A global user variable
-	      Item *item= nctx->get_item(i);
-	      Item_func_set_user_var *suv;
-	      Item_func_get_user_var *guv=
-		static_cast<Item_func_get_user_var*>(fi);
-
-	      suv= new Item_func_set_user_var(guv->get_name(), item);
-              /*
-                we do not check suv->fixed, because it can't be fixed after
-                creation
-              */
-	      suv->fix_fields(thd, NULL, &item);
-	      suv->fix_length_and_dec();
-	      suv->check();
-	      suv->update();
-	    }
+	    Item *item= nctx->get_item(i);
+	    Item_func_set_user_var *suv;
+
+	    suv= new Item_func_set_user_var(guv->get_name(), item);
+	    /*
+	      we do not check suv->fixed, because it can't be fixed after
+	      creation
+	    */
+	    suv->fix_fields(thd, NULL, &item);
+	    suv->fix_length_and_dec();
+	    suv->check();
+	    suv->update();
 	  }
 	}
       }
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index d98cdfdd226..def38009eee 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -34,7 +34,6 @@ sp_rcontext::sp_rcontext(uint fsize, uint hmax, uint cmax)
 {
   in_handler= FALSE;
   m_frame= (Item **)sql_alloc(fsize * sizeof(Item*));
-  m_outs= (int *)sql_alloc(fsize * sizeof(int));
   m_handler= (sp_handler_t *)sql_alloc(hmax * sizeof(sp_handler_t));
   m_hstack= (uint *)sql_alloc(hmax * sizeof(uint));
   m_cstack= (sp_cursor **)sql_alloc(cmax * sizeof(sp_cursor *));
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index 37d718048a0..afcd937a369 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -83,18 +83,6 @@ class sp_rcontext : public Sql_alloc
   }
 
   inline void
-  set_oindex(uint idx, int oidx)
-  {
-    m_outs[idx] = oidx;
-  }
-
-  inline int
-  get_oindex(uint idx)
-  {
-    return m_outs[idx];
-  }
-
-  inline void
   set_result(Item *it)
   {
     m_result= it;
@@ -187,7 +175,6 @@ private:
   uint m_count;
   uint m_fsize;
   Item **m_frame;
-  int  *m_outs;
 
   Item *m_result;		// For FUNCTIONs