From ec19e48021e408edac2e875cfeeef8f496a04d22 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Wed, 22 Mar 2017 18:10:33 +0400 Subject: [PATCH] MDEV-12314 Implicit cursor FOR LOOP for cursors with parameters --- .../compat/oracle/r/sp-cursor-rowtype.result | 29 +++++++++++++ .../compat/oracle/t/sp-cursor-rowtype.test | 28 +++++++++++++ sql/item_func.h | 4 ++ sql/sp_head.cc | 40 ++++++++++++++++-- sql/sp_head.h | 36 +++++++++++++--- sql/sql_lex.cc | 42 +++++++++++++++---- sql/sql_lex.h | 4 +- 7 files changed, 167 insertions(+), 16 deletions(-) diff --git a/mysql-test/suite/compat/oracle/r/sp-cursor-rowtype.result b/mysql-test/suite/compat/oracle/r/sp-cursor-rowtype.result index 04bb4298adaab..288d4492c843f 100644 --- a/mysql-test/suite/compat/oracle/r/sp-cursor-rowtype.result +++ b/mysql-test/suite/compat/oracle/r/sp-cursor-rowtype.result @@ -1134,6 +1134,35 @@ DROP TABLE t3; DROP TABLE t2; DROP TABLE t1; # +# MDEV-12314 Implicit cursor FOR LOOP for cursors with parameters +# +CREATE TABLE t1 (a INT, b VARCHAR(32)); +INSERT INTO t1 VALUES (10,'b0'); +INSERT INTO t1 VALUES (11,'b1'); +INSERT INTO t1 VALUES (12,'b2'); +CREATE PROCEDURE p1(pa INT, pb VARCHAR(32)) AS +CURSOR cur(va INT, vb VARCHAR(32)) IS +SELECT a, b FROM t1 WHERE a=va AND b=vb; +BEGIN +FOR rec IN cur(pa,pb) +LOOP +SELECT rec.a, rec.b; +END LOOP; +END; +$$ +CALL p1(10,'B0'); +rec.a rec.b +10 b0 +CALL p1(11,'B1'); +rec.a rec.b +11 b1 +CALL p1(12,'B2'); +rec.a rec.b +12 b2 +CALL p1(12,'non-existing'); +DROP TABLE t1; +DROP PROCEDURE p1; +# # MDEV-12098 sql_mode=ORACLE: Implicit cursor FOR loop # # Parse error in the cursor SELECT statement diff --git a/mysql-test/suite/compat/oracle/t/sp-cursor-rowtype.test b/mysql-test/suite/compat/oracle/t/sp-cursor-rowtype.test index 94db6b49cf41f..3c37a732d8c73 100644 --- a/mysql-test/suite/compat/oracle/t/sp-cursor-rowtype.test +++ b/mysql-test/suite/compat/oracle/t/sp-cursor-rowtype.test @@ -1230,6 +1230,34 @@ DROP TABLE t2; DROP TABLE t1; +--echo # +--echo # MDEV-12314 Implicit cursor FOR LOOP for cursors with parameters +--echo # + +CREATE TABLE t1 (a INT, b VARCHAR(32)); +INSERT INTO t1 VALUES (10,'b0'); +INSERT INTO t1 VALUES (11,'b1'); +INSERT INTO t1 VALUES (12,'b2'); +DELIMITER $$; +CREATE PROCEDURE p1(pa INT, pb VARCHAR(32)) AS + CURSOR cur(va INT, vb VARCHAR(32)) IS + SELECT a, b FROM t1 WHERE a=va AND b=vb; +BEGIN + FOR rec IN cur(pa,pb) + LOOP + SELECT rec.a, rec.b; + END LOOP; +END; +$$ +DELIMITER ;$$ +CALL p1(10,'B0'); +CALL p1(11,'B1'); +CALL p1(12,'B2'); +CALL p1(12,'non-existing'); +DROP TABLE t1; +DROP PROCEDURE p1; + + --echo # --echo # MDEV-12098 sql_mode=ORACLE: Implicit cursor FOR loop --echo # diff --git a/sql/item_func.h b/sql/item_func.h index 5b39c2ef0d86b..9d948a0334312 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -2641,6 +2641,10 @@ class Item_func_sp :public Item_func { return sp_result_field; } + const sp_name *get_sp_name() const + { + return m_name; + } bool check_vcol_func_processor(void *arg); bool limit_index_condition_pushdown_processor(void *opt_arg) diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 16dfbe9b700b6..20a0467be710d 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -4612,14 +4612,16 @@ Item *sp_head::adjust_assignment_source(THD *thd, Item *val, Item *val2) bool sp_head::set_local_variable(THD *thd, sp_pcontext *spcont, - sp_variable *spv, Item *val, LEX *lex) + sp_variable *spv, Item *val, LEX *lex, + bool responsible_to_free_lex) { if (!(val= adjust_assignment_source(thd, val, spv->default_value))) return true; sp_instr_set *sp_set= new (thd->mem_root) sp_instr_set(instructions(), spcont, - spv->offset, val, lex, true); + spv->offset, val, lex, + responsible_to_free_lex); return sp_set == NULL || add_instr(sp_set); } @@ -4689,8 +4691,15 @@ bool sp_head::add_open_cursor(THD *thd, sp_pcontext *spcont, uint offset, bool sp_head::add_for_loop_open_cursor(THD *thd, sp_pcontext *spcont, sp_variable *index, - const sp_pcursor *pcursor, uint coffset) + const sp_pcursor *pcursor, uint coffset, + sp_assignment_lex *param_lex, + Item_args *parameters) { + if (parameters && + add_set_for_loop_cursor_param_variables(thd, pcursor->param_context(), + param_lex, parameters)) + return true; + sp_instr *instr_copy_struct= new (thd->mem_root) sp_instr_cursor_copy_struct(instructions(), spcont, pcursor->lex(), @@ -4711,3 +4720,28 @@ bool sp_head::add_for_loop_open_cursor(THD *thd, sp_pcontext *spcont, instr_cfetch->add_to_varlist(index); return false; } + + +bool +sp_head::add_set_for_loop_cursor_param_variables(THD *thd, + sp_pcontext *param_spcont, + sp_assignment_lex *param_lex, + Item_args *parameters) +{ + DBUG_ASSERT(param_spcont->context_var_count() == parameters->argument_count()); + for (uint idx= 0; idx < parameters->argument_count(); idx ++) + { + /* + param_lex is shared between multiple items (cursor parameters). + Only the last sp_instr_set is responsible for freeing param_lex. + See more comments in LEX::sp_for_loop_cursor_declarations in sql_lex.cc. + */ + bool last= idx + 1 == parameters->argument_count(); + sp_variable *spvar= param_spcont->find_context_variable(idx); + if (set_local_variable(thd, param_spcont, + spvar, parameters->arguments()[idx], + param_lex, last)) + return true; + } + return false; +} diff --git a/sql/sp_head.h b/sql/sp_head.h index d8709feb08411..3dbed697f75bb 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -363,8 +363,20 @@ class sp_head :private Query_arena, } Item *adjust_assignment_source(THD *thd, Item *val, Item *val2); + /** + @param thd - the current thd + @param spcont - the current parse context + @param spv - the SP variable + @param val - the value to be assigned to the variable + @param lex - the LEX that was used to create "val" + @param responsible_to_free_lex - if the generated sp_instr_set should + free "lex". + @retval true - on error + @retval false - on success + */ bool set_local_variable(THD *thd, sp_pcontext *spcont, - sp_variable *spv, Item *val, LEX *lex); + sp_variable *spv, Item *val, LEX *lex, + bool responsible_to_free_lex); bool set_local_variable_row_field(THD *thd, sp_pcontext *spcont, sp_variable *spv, uint field_idx, Item *val, LEX *lex); @@ -394,7 +406,7 @@ class sp_head :private Query_arena, */ DBUG_ASSERT(m_thd->free_list == NULL); m_thd->free_list= prm->get_free_list(); - if (set_local_variable(thd, param_spcont, spvar, prm->get_item(), prm)) + if (set_local_variable(thd, param_spcont, spvar, prm->get_item(), prm, true)) return true; /* Safety: @@ -429,6 +441,15 @@ class sp_head :private Query_arena, return false; } + /** + Generate a code to set all cursor parameter variables for a FOR LOOP, e.g.: + FOR index IN cursor(1,2,3) + @param + */ + bool add_set_for_loop_cursor_param_variables(THD *thd, + sp_pcontext *param_spcont, + sp_assignment_lex *param_lex, + Item_args *parameters); public: /** @@ -451,9 +472,11 @@ class sp_head :private Query_arena, /** Generate an initiation code for a CURSOR FOR LOOP, e.g.: - FOR index IN cursor + FOR index IN cursor -- cursor without parameters + FOR index IN cursor(1,2,3) -- cursor with parameters The code generated by this method does the following during SP run-time: + - Sets all cursor parameter vartiables from "parameters" - Initializes the index ROW-type variable from the cursor (the structure is copied from the cursor to the index variable) - The cursor gets opened @@ -464,13 +487,16 @@ class sp_head :private Query_arena, @param index - the loop "index" ROW-type variable @param pcursor - the cursor @param coffset - the cursor offset + @param param_lex - the LEX that owns Items in "parameters" + @param parameters - the cursor parameters Item array @retval true - on error (EOM) @retval false - on success */ bool add_for_loop_open_cursor(THD *thd, sp_pcontext *spcont, sp_variable *index, - const sp_pcursor *pcursor, uint coffset); - + const sp_pcursor *pcursor, uint coffset, + sp_assignment_lex *param_lex, + Item_args *parameters); /** Returns true if any substatement in the routine directly (not through another routine) modifies data/changes table. diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 7e5d29cd67518..5f764f625f65b 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -5436,7 +5436,9 @@ sp_variable * LEX::sp_add_for_loop_cursor_variable(THD *thd, const LEX_STRING name, const sp_pcursor *pcursor, - uint coffset) + uint coffset, + sp_assignment_lex *param_lex, + Item_args *parameters) { sp_variable *spvar= spcont->add_variable(thd, name); spcont->declare_var_boundary(1); @@ -5448,7 +5450,8 @@ LEX::sp_add_for_loop_cursor_variable(THD *thd, return NULL; spvar->field_def.set_cursor_rowtype_ref(ref); - if (sphead->add_for_loop_open_cursor(thd, spcont, spvar, pcursor, coffset)) + if (sphead->add_for_loop_open_cursor(thd, spcont, spvar, pcursor, coffset, + param_lex, parameters)) return NULL; spcont->declare_var_boundary(0); @@ -5538,8 +5541,9 @@ bool LEX::sp_for_loop_cursor_declarations(THD *thd, Item *item= bounds.m_index->get_item(); Item_splocal *item_splocal; Item_field *item_field; + Item_func_sp *item_func_sp= NULL; LEX_STRING name; - uint coffs; + uint coffs, param_count= 0; const sp_pcursor *pcursor; if ((item_splocal= item->get_item_splocal())) @@ -5553,16 +5557,40 @@ bool LEX::sp_for_loop_cursor_declarations(THD *thd, name.str= (char *) item_field->field_name; name.length= strlen(item_field->field_name); } + else if (item->type() == Item::FUNC_ITEM && + static_cast(item)->functype() == Item_func::FUNC_SP && + !static_cast(item)->get_sp_name()->m_explicit_name) + { + /* + When a FOR LOOP for a cursor with parameters is parsed: + FOR index IN cursor(1,2,3) LOOP + statements; + END LOOP; + the parser scans "cursor(1,2,3)" using the "expr" rule, + so it thinks that cursor(1,2,3) is a stored function call. + It's not easy to implement this without using "expr" because + of grammar conflicts. + As a side effect, the Item_func_sp and its arguments in the parentheses + belong to the same LEX. This is different from an explicit + "OPEN cursor(1,2,3)" where every expression belongs to a separate LEX. + */ + item_func_sp= static_cast(item); + name= item_func_sp->get_sp_name()->m_name; + param_count= item_func_sp->argument_count(); + } else { thd->parse_error(); return true; } - if (!(pcursor= spcont->find_cursor_with_error(name, &coffs, false))) + if (!(pcursor= spcont->find_cursor_with_error(name, &coffs, false)) || + pcursor->check_param_count_with_error(param_count)) return true; if (!(loop->m_index= sp_add_for_loop_cursor_variable(thd, index, - pcursor, coffs))) + pcursor, coffs, + bounds.m_index, + item_func_sp))) return true; loop->m_upper_bound= NULL; loop->m_direction= bounds.m_direction; @@ -5590,7 +5618,7 @@ bool LEX::sp_for_loop_increment(THD *thd, const Lex_for_loop_st &loop) return true; Item *expr= new (thd->mem_root) Item_func_plus(thd, splocal, inc); if (!expr || - sphead->set_local_variable(thd, spcont, loop.m_index, expr, this)) + sphead->set_local_variable(thd, spcont, loop.m_index, expr, this, true)) return true; return false; } @@ -6464,7 +6492,7 @@ bool LEX::set_variable(struct sys_var_with_base *variable, Item *item) sp_variable *spv= spcont->find_variable(variable->base_name, false); DBUG_ASSERT(spv); /* It is a local variable. */ - return sphead->set_local_variable(thd, spcont, spv, item, this); + return sphead->set_local_variable(thd, spcont, spv, item, this, true); } diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 8afc02a099cfb..fe07c0367c968 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -3376,7 +3376,9 @@ struct LEX: public Query_tables_list sp_variable *sp_add_for_loop_cursor_variable(THD *thd, const LEX_STRING name, const class sp_pcursor *cur, - uint coffset); + uint coffset, + sp_assignment_lex *param_lex, + Item_args *parameters); bool sp_for_loop_cursor_condition_test(THD *thd, const Lex_for_loop_st &loop); bool sp_for_loop_cursor_finalize(THD *thd, const Lex_for_loop_st &);