MDEV-17154 Multiple selects from parametrized CTE fails with syntax e…

…rror

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

@@ -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;

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;

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

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

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)