MDEV-12314 Implicit cursor FOR LOOP for cursors with parameters

Author: Alexander Barkov

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

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 #

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)

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

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.

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_func*>(item)->functype() == Item_func::FUNC_SP &&
+           !static_cast<Item_func_sp*>(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_func_sp*>(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);
 }
 
 

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