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

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

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

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

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;

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

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