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
