revision-id: 39c4fe1eb6f7ed73ec847ba294184d478e5a384e (mariadb-10.2.16-126-g39c4fe1) parent(s): 4d991abd4fc7f60e758ec46301b0dd2bee71245c author: Igor Babaev committer: Igor Babaev timestamp: 2018-09-13 00:35:28 -0700 message: MDEV-17154 Multiple selects from parametrized CTE fails with syntax error This patch fills a serious flaw in the implementation of common table expressions. Before this patch an attempt to prepare a statement from a query with a parameter marker in a CTE that was used more than once in the query ended up with a bogus error message. Similarly if a statement in a stored procedure contained a CTE whose specification used a local variables and this CTE was referred to more than once in the statement then the server failed to execute the stored procedure returning a bogus error message on a non-existing field. The problems appeared due to incorrect handling of parameter markers / local variables in CTEs that were referred more than once. This patch fixes the problems by differentiating between the original occurrences of a parameter marker / local variable used in the specification of a CTE and the corresponding occurrences used in copies of this specification. These copies are substituted instead of non-first references to the CTE. The idea of the fix and even some code were taken from the MySQL implementation of the common table expressions. --- mysql-test/r/cte_nonrecursive.result | 129 +++++++++++++++++++++++++++++++++++ mysql-test/t/cte_nonrecursive.test | 90 ++++++++++++++++++++++++ sql/item.cc | 53 +++++++++++++- sql/item.h | 11 +++ sql/sql_cte.cc | 37 ++++++++-- sql/sql_cte.h | 8 ++- sql/sql_lex.cc | 1 + sql/sql_lex.h | 5 ++ sql/sql_prepare.cc | 12 ++++ sql/sql_yacc.yy | 61 +++++++++++++---- 10 files changed, 387 insertions(+), 20 deletions(-) diff --git a/mysql-test/r/cte_nonrecursive.result b/mysql-test/r/cte_nonrecursive.result index f6b8015..2b9455d 100644 --- a/mysql-test/r/cte_nonrecursive.result +++ b/mysql-test/r/cte_nonrecursive.result @@ -1512,3 +1512,132 @@ a a 1 1 drop database db_mdev_16473; use test; +# +# MDEV-17154: using parameter markers for PS within CTEs more than once +# using local variables in SP within CTEs more than once +# +prepare stmt from " +with cte(c) as (select ? ) select r.c, s.c+10 from cte as r, cte as s; +"; +set @a=2; +execute stmt using @a; +c s.c+10 +2 12 +set @a=5; +execute stmt using @a; +c s.c+10 +5 15 +deallocate prepare stmt; +prepare stmt from " +with cte(c) as (select ? ) select c from cte union select c+10 from cte; +"; +set @a=2; +execute stmt using @a; +c +2 +12 +set @a=5; +execute stmt using @a; +c +5 +15 +deallocate prepare stmt; +prepare stmt from " +with cte_e(a,b) as +( + with cte_o(c) as (select ?) + select r.c+10, s.c+20 from cte_o as r, cte_o as s +) +select * from cte_e as cte_e1 where a > 12 +union all +select * from cte_e as cte_e2; +"; +set @a=2; +execute stmt using @a; +a b +12 22 +set @a=5; +execute stmt using @a; +a b +15 25 +15 25 +deallocate prepare stmt; +create table t1 (a int, b int); +insert into t1 values +(3,33), (1,17), (7,72), (4,45), (2,27), (3,35), (4,47), (3,38), (2,22); +prepare stmt from " +with cte as (select * from t1 where a < ? and b > ?) + select r.a, r.b+10, s.a, s.b+20 from cte as r, cte as s where r.a=s.a+1; +"; +set @a=4, @b=20; +execute stmt using @a,@b; +a r.b+10 a s.b+20 +3 43 2 47 +3 45 2 47 +3 48 2 47 +3 43 2 42 +3 45 2 42 +3 48 2 42 +set @a=5, @b=20; +execute stmt using @a,@b; +a r.b+10 a s.b+20 +4 55 3 53 +4 57 3 53 +3 43 2 47 +3 45 2 47 +3 48 2 47 +4 55 3 55 +4 57 3 55 +4 55 3 58 +4 57 3 58 +3 43 2 42 +3 45 2 42 +3 48 2 42 +deallocate prepare stmt; +create procedure p1() +begin +declare i int; +set i = 0; +while i < 4 do +insert into t1 +with cte(a) as (select i) select r.a-1, s.a+1 from cte as r, cte as s; +set i = i+1; +end while; +end| +create procedure p2(in i int) +begin +insert into t1 +with cte(a) as (select i) select r.a-1, s.a+1 from cte as r, cte as s; +end| +delete from t1; +call p1(); +select * from t1; +a b +-1 1 +0 2 +1 3 +2 4 +call p1(); +select * from t1; +a b +-1 1 +0 2 +1 3 +2 4 +-1 1 +0 2 +1 3 +2 4 +delete from t1; +call p2(3); +select * from t1; +a b +2 4 +call p2(7); +select * from t1; +a b +2 4 +6 8 +drop procedure p1; +drop procedure p2; +drop table t1; diff --git a/mysql-test/t/cte_nonrecursive.test b/mysql-test/t/cte_nonrecursive.test index 11c864b..648fc89 100644 --- a/mysql-test/t/cte_nonrecursive.test +++ b/mysql-test/t/cte_nonrecursive.test @@ -1057,3 +1057,93 @@ select * from cte, db_mdev_16473.t1 as t where cte.a=t.a; drop database db_mdev_16473; use test; + +--echo # +--echo # MDEV-17154: using parameter markers for PS within CTEs more than once +--echo # using local variables in SP within CTEs more than once +--echo # + +prepare stmt from " +with cte(c) as (select ? ) select r.c, s.c+10 from cte as r, cte as s; +"; +set @a=2; +execute stmt using @a; +set @a=5; +execute stmt using @a; +deallocate prepare stmt; + +prepare stmt from " +with cte(c) as (select ? ) select c from cte union select c+10 from cte; +"; +set @a=2; +execute stmt using @a; +set @a=5; +execute stmt using @a; +deallocate prepare stmt; + +prepare stmt from " +with cte_e(a,b) as +( + with cte_o(c) as (select ?) + select r.c+10, s.c+20 from cte_o as r, cte_o as s +) +select * from cte_e as cte_e1 where a > 12 +union all +select * from cte_e as cte_e2; +"; +set @a=2; +execute stmt using @a; +set @a=5; +execute stmt using @a; +deallocate prepare stmt; + +create table t1 (a int, b int); +insert into t1 values + (3,33), (1,17), (7,72), (4,45), (2,27), (3,35), (4,47), (3,38), (2,22); + +prepare stmt from " +with cte as (select * from t1 where a < ? and b > ?) + select r.a, r.b+10, s.a, s.b+20 from cte as r, cte as s where r.a=s.a+1; +"; +set @a=4, @b=20; +execute stmt using @a,@b; +set @a=5, @b=20; +execute stmt using @a,@b; +deallocate prepare stmt; + +delimiter |; + +create procedure p1() +begin + declare i int; + set i = 0; + while i < 4 do + insert into t1 + with cte(a) as (select i) select r.a-1, s.a+1 from cte as r, cte as s; + set i = i+1; + end while; +end| + +create procedure p2(in i int) +begin + insert into t1 + with cte(a) as (select i) select r.a-1, s.a+1 from cte as r, cte as s; +end| + +delimiter ;| + +delete from t1; +call p1(); +select * from t1; +call p1(); +select * from t1; + +delete from t1; +call p2(3); +select * from t1; +call p2(7); +select * from t1; + +drop procedure p1; +drop procedure p2; +drop table t1; diff --git a/sql/item.cc b/sql/item.cc index 7d8baf8..38fef4a 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -3423,7 +3423,8 @@ Item_param::Item_param(THD *thd, uint pos_in_query_arg): For dynamic SQL, settability depends on the type of Item passed as an actual parameter. See Item_param::set_from_item(). */ - m_is_settable_routine_parameter(true) + m_is_settable_routine_parameter(true), + m_clones(thd->mem_root) { name= (char*) "?"; /* @@ -3435,6 +3436,56 @@ Item_param::Item_param(THD *thd, uint pos_in_query_arg): } +/* Add reference to Item_param used in a copy of CTE to its master as a clone */ + +bool Item_param::add_as_clone(THD *thd) +{ + LEX *lex= thd->lex; + uint master_pos= pos_in_query + lex->clone_spec_offset; + List_iterator_fast<Item_param> it(lex->param_list); + Item_param *master_param; + while ((master_param = it++)) + { + if (master_pos == master_param->pos_in_query) + return master_param->register_clone(this); + } + DBUG_ASSERT(false); + return false; +} + + +/* Update all clones of Item_param to sync their values with the item's value */ + +void Item_param::sync_clones() +{ + Item_param **c_ptr= m_clones.begin(); + Item_param **end= m_clones.end(); + for ( ; c_ptr < end; c_ptr++) + { + Item_param *c= *c_ptr; + /* Scalar-type members: */ + c->maybe_null= maybe_null; + c->null_value= null_value; + c->max_length= max_length; + c->decimals= decimals; + c->state= state; + c->item_type= item_type; + c->set_param_func= set_param_func; + c->value= value; + c->unsigned_flag= unsigned_flag; + /* Class-type members: */ + c->decimal_value= decimal_value; + /* + Note that String's assignment op properly sets m_is_alloced to 'false', + which is correct here: c->str_value doesn't own anything. + */ + c->str_value= str_value; + c->str_value_ptr= str_value_ptr; + c->collation= collation; + } +} + + void Item_param::set_null() { DBUG_ENTER("Item_param::set_null"); diff --git a/sql/item.h b/sql/item.h index 8fad8da..1a280e3 100644 --- a/sql/item.h +++ b/sql/item.h @@ -28,6 +28,7 @@ #include "field.h" /* Derivation */ #include "sql_type.h" #include "sql_time.h" +#include "mem_root_array.h" C_MODE_START #include <ma_dyncol.h> @@ -3067,6 +3068,10 @@ class Item_param :public Item_basic_value, bool check_vcol_func_processor(void *int_arg) {return FALSE;} Item *get_copy(THD *thd, MEM_ROOT *mem_root) { return 0; } + bool add_as_clone(THD *thd); + void sync_clones(); + bool register_clone(Item_param *i) { return m_clones.push_back(i); } + private: void invalid_default_param() const; @@ -3082,6 +3087,12 @@ class Item_param :public Item_basic_value, private: Send_field *m_out_param_info; bool m_is_settable_routine_parameter; + /* + Array of all references of this parameter marker used in a CTE to its clones + created for copies of this marker used the CTE's copies. It's used to + synchronize the actual value of the parameter with the values of the clones. + */ + Mem_root_array<Item_param *, true> m_clones; }; diff --git a/sql/sql_cte.cc b/sql/sql_cte.cc index 45f24c3..5a590bf 100644 --- a/sql/sql_cte.cc +++ b/sql/sql_cte.cc @@ -726,9 +726,10 @@ bool With_clause::prepare_unreferenced_elements(THD *thd) @brief Save the specification of the given with table as a string - @param thd The context of the statement containing this with element - @param spec_start The beginning of the specification in the input string - @param spec_end The end of the specification in the input string + @param thd The context of the statement containing this with element + @param spec_start The beginning of the specification in the input string + @param spec_end The end of the specification in the input string + @param spec_offset The offset of the specification in the input string @details The method creates for a string copy of the specification used in this @@ -740,11 +741,19 @@ bool With_clause::prepare_unreferenced_elements(THD *thd) true on failure */ -bool With_element::set_unparsed_spec(THD *thd, char *spec_start, char *spec_end) +bool With_element::set_unparsed_spec(THD *thd, char *spec_start, char *spec_end, + uint spec_offset) { + stmt_prepare_mode= thd->m_parser_state->m_lip.stmt_prepare_mode; unparsed_spec.length= spec_end - spec_start; - unparsed_spec.str= (char*) thd->memdup(spec_start, unparsed_spec.length+1); - unparsed_spec.str[unparsed_spec.length]= '\0'; + if (stmt_prepare_mode || !thd->lex->sphead) + unparsed_spec.str= spec_start; + else + { + unparsed_spec.str= (char*) thd->memdup(spec_start, unparsed_spec.length+1); + unparsed_spec.str[unparsed_spec.length]= '\0'; + } + unparsed_spec_offset= spec_offset; if (!unparsed_spec.str) { @@ -814,13 +823,28 @@ st_select_lex_unit *With_element::clone_parsed_spec(THD *thd, TABLE_LIST *spec_tables_tail; st_select_lex *with_select; + char save_end= unparsed_spec.str[unparsed_spec.length]; + unparsed_spec.str[unparsed_spec.length]= '\0'; if (parser_state.init(thd, unparsed_spec.str, unparsed_spec.length)) goto err; + parser_state.m_lip.stmt_prepare_mode= stmt_prepare_mode; + parser_state.m_lip.multi_statements= false; + parser_state.m_lip.m_digest= NULL; + lex_start(thd); + lex->clone_spec_offset= unparsed_spec_offset; + lex->param_list= old_lex->param_list; + lex->sphead= old_lex->sphead; + lex->spname= old_lex->spname; + lex->spcont= old_lex->spcont; + lex->sp_chistics= old_lex->sp_chistics; + lex->stmt_lex= old_lex; with_select= &lex->select_lex; with_select->select_number= ++thd->lex->stmt_lex->current_select_number; parse_status= parse_sql(thd, &parser_state, 0); + unparsed_spec.str[unparsed_spec.length]= save_end; + if (parse_status) goto err; @@ -865,6 +889,7 @@ st_select_lex_unit *With_element::clone_parsed_spec(THD *thd, with_select)); if (check_dependencies_in_with_clauses(lex->with_clauses_list)) res= NULL; + lex->sphead= NULL; // in order not to delete lex->sphead lex_end(lex); err: if (arena) diff --git a/sql/sql_cte.h b/sql/sql_cte.h index 6351b65..58f371d 100644 --- a/sql/sql_cte.h +++ b/sql/sql_cte.h @@ -74,6 +74,11 @@ class With_element : public Sql_alloc It used to build clones of the specification if they are needed. */ LEX_STRING unparsed_spec; + /* Offset of the specification in the input string */ + uint unparsed_spec_offset; + + /* True if the with element is used a prepared statement */ + bool stmt_prepare_mode; /* Return the map where 1 is set only in the position for this element */ table_map get_elem_map() { return (table_map) 1 << number; } @@ -174,7 +179,8 @@ class With_element : public Sql_alloc TABLE_LIST *find_first_sq_rec_ref_in_select(st_select_lex *sel); - bool set_unparsed_spec(THD *thd, char *spec_start, char *spec_end); + bool set_unparsed_spec(THD *thd, char *spec_start, char *spec_end, + uint spec_offset); st_select_lex_unit *clone_parsed_spec(THD *thd, TABLE_LIST *with_table); diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index d3ddd9e..3624b23 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -671,6 +671,7 @@ void lex_start(THD *thd) lex->curr_with_clause= 0; lex->with_clauses_list= 0; lex->with_clauses_list_last_next= &lex->with_clauses_list; + lex->clone_spec_offset= 0; lex->value_list.empty(); lex->update_list.empty(); lex->set_var_list.empty(); diff --git a/sql/sql_lex.h b/sql/sql_lex.h index a1f6b20..939b48ba 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -2552,6 +2552,11 @@ struct LEX: public Query_tables_list with clause in the current statement */ With_clause **with_clauses_list_last_next; + /* + When a copy of a with element is parsed this is set to the offset of + the with element in the input string, otherwise it's set to 0 + */ + uint clone_spec_offset; /* Query Plan Footprint of a currently running select */ Explain_query *explain; diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 0d7b043..51e9152 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -947,6 +947,7 @@ static bool insert_params(Prepared_statement *stmt, uchar *null_array, DBUG_RETURN(1); if (param->convert_str_value(stmt->thd)) DBUG_RETURN(1); /* out of memory */ + param->sync_clones(); } DBUG_RETURN(0); } @@ -995,6 +996,7 @@ static bool insert_bulk_params(Prepared_statement *stmt, } else DBUG_RETURN(1); // long is not supported here + param->sync_clones(); } DBUG_RETURN(0); } @@ -1023,6 +1025,7 @@ static bool set_conversion_functions(Prepared_statement *stmt, read_pos+= 2; (**it).unsigned_flag= MY_TEST(typecode & signed_bit); setup_one_conversion_function(thd, *it, (uchar) (typecode & 0xff)); + (*it)->sync_clones(); } *data= read_pos; DBUG_RETURN(0); @@ -1093,6 +1096,7 @@ static bool emb_insert_params(Prepared_statement *stmt, String *expanded_query) if (param->has_no_value()) DBUG_RETURN(1); } + param->sync_clones(); } if (param->convert_str_value(thd)) DBUG_RETURN(1); /* out of memory */ @@ -1135,6 +1139,7 @@ static bool emb_insert_params_with_log(Prepared_statement *stmt, String *query) if (param->convert_str_value(thd)) DBUG_RETURN(1); /* out of memory */ + param->sync_clones(); } if (acc.finalize()) DBUG_RETURN(1); @@ -1190,7 +1195,11 @@ swap_parameter_array(Item_param **param_array_dst, Item_param **end= param_array_dst + param_count; for (; dst < end; ++src, ++dst) + { (*dst)->set_param_type_and_swap_value(*src); + (*dst)->sync_clones(); + (*src)->sync_clones(); + } } @@ -1221,6 +1230,7 @@ insert_params_from_actual_params(Prepared_statement *stmt, if (ps_param->save_in_param(stmt->thd, param) || param->convert_str_value(stmt->thd)) DBUG_RETURN(1); + param->sync_clones(); } DBUG_RETURN(0); } @@ -1269,6 +1279,8 @@ insert_params_from_actual_params_with_log(Prepared_statement *stmt, if (param->convert_str_value(thd)) DBUG_RETURN(1); + + param->sync_clones(); } if (acc.finalize()) DBUG_RETURN(1); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 47e5f2f..6e7128c 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1751,7 +1751,8 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize); table_ident_opt_wild create_like %type <simple_string> - remember_name remember_end opt_db remember_tok_start + remember_name remember_end opt_db + remember_tok_start remember_tok_end wild_and_where field_length opt_field_length opt_field_length_default_1 @@ -8750,6 +8751,12 @@ remember_tok_start: } ; +remember_tok_end: + { + $$= (char*) YYLIP->get_tok_end(); + } + ; + remember_name: { $$= (char*) YYLIP->get_cpp_tok_start(); @@ -11828,10 +11835,18 @@ limit_option: sp_pcontext *spc = lex->spcont; if (spc && (spv = spc->find_variable($1, false))) { + uint pos_in_query= 0; + uint len_in_query= 0; + if (!lex->clone_spec_offset) + { + pos_in_query= (uint)(lip->get_tok_start() - + lex->sphead->m_tmp_query); + len_in_query= (uint)(lip->get_ptr() - + lip->get_tok_start()); + } splocal= new (thd->mem_root) Item_splocal(thd, $1, spv->offset, spv->sql_type(), - (uint)(lip->get_tok_start() - lex->sphead->m_tmp_query), - (uint)(lip->get_ptr() - lip->get_tok_start())); + pos_in_query, len_in_query); if (splocal == NULL) MYSQL_YYABORT; #ifndef DBUG_OFF @@ -13842,14 +13857,23 @@ param_marker: LEX *lex= thd->lex; Lex_input_stream *lip= YYLIP; Item_param *item; + bool rc; if (! lex->parsing_options.allows_variable) my_yyabort_error((ER_VIEW_SELECT_VARIABLE, MYF(0))); - const char *query_start= lex->sphead ? lex->sphead->m_tmp_query - : thd->query(); - item= new (thd->mem_root) Item_param(thd, (uint)(lip->get_tok_start() - - query_start)); - if (!($$= item) || lex->param_list.push_back(item, thd->mem_root)) + const char *query_start= lex->sphead && !lex->clone_spec_offset ? + lex->sphead->m_tmp_query : lip->get_buf(); + item= new (thd->mem_root) Item_param(thd, + (uint)(lip->get_tok_start() - + query_start)); + if (!($$= item)) + MYSQL_YYABORT; + if (!lex->clone_spec_offset) + rc= lex->param_list.push_back(item, thd->mem_root); + else + rc= item->add_as_clone(thd); + if (rc) my_yyabort_error((ER_OUT_OF_RESOURCES, MYF(0))); + } ; @@ -14045,12 +14069,17 @@ with_list_element: MYSQL_YYABORT; Lex->with_column_list.empty(); } - AS '(' remember_name subselect remember_end ')' + AS '(' remember_tok_start subselect remember_tok_end ')' { + LEX *lex= thd->lex; + const char *query_start= lex->sphead ? lex->sphead->m_tmp_query + : thd->query(); + char *spec_start= $6 + 1; With_element *elem= new With_element($1, *$2, $7->master_unit()); if (elem == NULL || Lex->curr_with_clause->add_with_element(elem)) MYSQL_YYABORT; - if (elem->set_unparsed_spec(thd, $6+1, $8)) + if (elem->set_unparsed_spec(thd, spec_start, $8, + spec_start - query_start)) MYSQL_YYABORT; } ; @@ -14140,10 +14169,18 @@ simple_ident: my_yyabort_error((ER_VIEW_SELECT_VARIABLE, MYF(0))); Item_splocal *splocal; + uint pos_in_query= 0; + uint len_in_query= 0; + if (!lex->clone_spec_offset) + { + pos_in_query= (uint)(lip->get_tok_start_prev() - + lex->sphead->m_tmp_query); + len_in_query= (uint)(lip->get_tok_end() - + lip->get_tok_start_prev()); + } splocal= new (thd->mem_root) Item_splocal(thd, $1, spv->offset, spv->sql_type(), - (uint)(lip->get_tok_start_prev() - lex->sphead->m_tmp_query), - (uint)(lip->get_tok_end() - lip->get_tok_start_prev())); + pos_in_query, len_in_query); if (splocal == NULL) MYSQL_YYABORT; #ifndef DBUG_OFF