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

@@ -125,13 +125,13 @@ set @x = x|
 create function f(x int) returns int
 return x+42|
 call p()|
-ERROR 42000: Incorrect number of arguments for PROCEDURE p; expected 1, got 0
+ERROR 42000: Incorrect number of arguments for PROCEDURE test.p; expected 1, got 0
 call p(1, 2)|
-ERROR 42000: Incorrect number of arguments for PROCEDURE p; expected 1, got 2
+ERROR 42000: Incorrect number of arguments for PROCEDURE test.p; expected 1, got 2
 select f()|
-ERROR 42000: Incorrect number of arguments for FUNCTION f; expected 1, got 0
+ERROR 42000: Incorrect number of arguments for FUNCTION test.f; expected 1, got 0
 select f(1, 2)|
-ERROR 42000: Incorrect number of arguments for FUNCTION f; expected 1, got 2
+ERROR 42000: Incorrect number of arguments for FUNCTION test.f; expected 1, got 2
 drop procedure p|
 drop function f|
 create procedure p(val int, out res int)
@@ -318,6 +318,24 @@ select field from t1;
 label L1;
 end|
 ERROR HY000: GOTO is not allowed in a stored procedure handler
+drop procedure if exists p|
+create procedure p(in x int, inout y int, out z int)
+begin
+set y = x+y;
+set z = x+y;
+end|
+set @tmp_x = 42|
+set @tmp_y = 3|
+set @tmp_z = 0|
+call p(@tmp_x, @tmp_y, @tmp_z)|
+select @tmp_x, @tmp_y, @tmp_z|
+@tmp_x	@tmp_y	@tmp_z
+42	45	87
+call p(42, 43, @tmp_z)|
+ERROR 42000: OUT or INOUT argument 2 for routine test.p is not a variable
+call p(42, @tmp_y, 43)|
+ERROR 42000: OUT or INOUT argument 3 for routine test.p is not a variable
+drop procedure p|
 create procedure bug1965()
 begin
 declare c cursor for select val from t1 order by valname;

mysql-test/r/sp.result

@@ -2908,4 +2908,32 @@ v/10
 10.00000
 drop procedure bug9674_1|
 drop procedure bug9674_2|
+drop procedure if exists bug9598_1|
+drop procedure if exists bug9598_2|
+create procedure bug9598_1(in var_1 char(16),
+out var_2 integer, out var_3 integer)
+begin
+set var_2 = 50;
+set var_3 = 60;
+end|
+create procedure bug9598_2(in v1 char(16),
+in v2 integer,
+in v3 integer,
+in v4 integer,
+in v5 integer)
+begin
+select v1,v2,v3,v4,v5;
+call bug9598_1(v1,@tmp1,@tmp2);
+select v1,v2,v3,v4,v5;
+end|
+call bug9598_2('Test',2,3,4,5)|
+v1	v2	v3	v4	v5
+Test	2	3	4	5
+v1	v2	v3	v4	v5
+Test	2	3	4	5
+select @tmp1, @tmp2|
+@tmp1	@tmp2
+50	60
+drop procedure bug9598_1|
+drop procedure bug9598_2|
 drop table t1,t2;

mysql-test/t/sp-error.test

@@ -410,6 +410,31 @@ begin
   label L1;
 end|
 
+# Check in and inout arguments.
+--disable_warnings
+drop procedure if exists p|
+--enable_warnings
+create procedure p(in x int, inout y int, out z int)
+begin
+  set y = x+y;
+  set z = x+y;
+end|
+
+set @tmp_x = 42|
+set @tmp_y = 3|
+set @tmp_z = 0|
+# For reference: this is ok
+call p(@tmp_x, @tmp_y, @tmp_z)|
+select @tmp_x, @tmp_y, @tmp_z|
+
+--error ER_SP_NOT_VAR_ARG
+call p(42, 43, @tmp_z)|
+--error ER_SP_NOT_VAR_ARG
+call p(42, @tmp_y, 43)|
+
+drop procedure p|
+
+
 #
 # BUG#1965
 #

mysql-test/t/sp.test

@@ -3571,6 +3571,38 @@ drop procedure bug9674_1|
 drop procedure bug9674_2|
 
 
+#
+# BUG#9598: stored procedure call within stored procedure overwrites IN variable
+#
+--disable_warnings
+drop procedure if exists bug9598_1|
+drop procedure if exists bug9598_2|
+--enable_warnings
+create procedure bug9598_1(in var_1 char(16),
+                           out var_2 integer, out var_3 integer)
+begin
+  set var_2 = 50;
+  set var_3 = 60;
+end|
+
+create procedure bug9598_2(in v1 char(16),
+                           in v2 integer,
+                           in v3 integer,
+                           in v4 integer,
+                           in v5 integer)
+begin
+  select v1,v2,v3,v4,v5;
+  call bug9598_1(v1,@tmp1,@tmp2);
+  select v1,v2,v3,v4,v5;
+end|
+
+call bug9598_2('Test',2,3,4,5)|
+select @tmp1, @tmp2|
+
+drop procedure bug9598_1|
+drop procedure bug9598_2|
+
+
 #
 # BUG#NNNN: New bug synopsis
 #

sql/item.h

@@ -545,6 +545,8 @@ class Item {
     cleanup();
     delete this;
   }
+
+  virtual bool is_splocal() { return 0; } /* Needed for error checking */
 };
 
 
@@ -564,6 +566,8 @@ class Item_splocal : public Item
     Item::maybe_null= TRUE;
   }
 
+  bool is_splocal() { return 1; } /* Needed for error checking */
+
   Item *this_item();
   Item *this_const_item() const;
 

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"

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();
 	  }
 	}
       }

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 *));

sql/sp_rcontext.h

@@ -82,18 +82,6 @@ class sp_rcontext : public Sql_alloc
     return m_frame[idx];
   }
 
-  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)
   {
@@ -187,7 +175,6 @@ class sp_rcontext : public Sql_alloc
   uint m_count;
   uint m_fsize;
   Item **m_frame;
-  int  *m_outs;
 
   Item *m_result;		// For FUNCTIONs