From 6aad6835c62d4bdb03047ee10f7864c73942bc1f Mon Sep 17 00:00:00 2001 From: "pem@mysql.comhem.se" <> Date: Thu, 14 Apr 2005 14:52:35 +0200 Subject: [PATCH] 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 | 26 +++++++++-- mysql-test/r/sp.result | 28 ++++++++++++ mysql-test/t/sp-error.test | 25 +++++++++++ mysql-test/t/sp.test | 32 ++++++++++++++ sql/item.h | 4 ++ sql/share/errmsg.txt | 2 + sql/sp_head.cc | 86 +++++++++++++++++++----------------- sql/sp_rcontext.cc | 1 - sql/sp_rcontext.h | 13 ------ 9 files changed, 159 insertions(+), 58 deletions(-) diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result index b9d33bbe606..3f224d790f7 100644 --- a/mysql-test/r/sp-error.result +++ b/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; diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index cac847fd602..99d5e82a83b 100644 --- a/mysql-test/r/sp.result +++ b/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; diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test index a02d2ff6733..ecbc98f86e9 100644 --- a/mysql-test/t/sp-error.test +++ b/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 # diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 866db5c0191..ad6f5406a16 100644 --- a/mysql-test/t/sp.test +++ b/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 # 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(it); + + if (fi->functype() == Item_func::GUSERVAR_FUNC) + return static_cast(fi); + } + return NULL; +} + int sp_head::execute_procedure(THD *thd, List *args) { @@ -708,7 +721,7 @@ sp_head::execute_procedure(THD *thd, List *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 *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 *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 *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(it)->get_offset()); } } @@ -786,38 +799,31 @@ sp_head::execute_procedure(THD *thd, List *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(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(it); + Item *item= nctx->get_item(i); + Item_func_set_user_var *suv; - 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(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(); - } + 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 @@ -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 @@ private: uint m_count; uint m_fsize; Item **m_frame; - int *m_outs; Item *m_result; // For FUNCTIONs