A cleanup in sp_rcontext, as requested by Monty

- Changing sp_rcontext::m_var_items from list of Item to list of Item_field
- Renaming sp_rcontext::get_item() to get_variable() and changing
  its return type from Item* to Item_field *
- Adding sp_rcontext::get_parameter() and sp_rcontext::set_parameter(),
  wrappers for get_variable() and set_variable() with extra DBUG_ASSERT.
  Using new methods instead of get_variable()/set_variable() in
  relevant places.

5 files changed, 45 insertions, 27 deletions

diff --git a/sql/item.cc b/sql/item.cc
index 3668278d4c7..d4da3f866d0 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -1788,7 +1788,7 @@ Item_splocal::Item_splocal(THD *thd, const LEX_CSTRING *sp_var_name,
 
 bool Item_splocal::fix_fields(THD *thd, Item **ref)
 {
-  Item *item= thd->spcont->get_item(m_var_idx);
+  Item_field *item= thd->spcont->get_variable(m_var_idx);
   set_handler(item->type_handler());
   return fix_fields_from_item(thd, ref, item);
 }
@@ -1799,7 +1799,7 @@ Item_splocal::this_item()
 {
   DBUG_ASSERT(m_sp == m_thd->spcont->m_sp);
   DBUG_ASSERT(fixed);
-  return m_thd->spcont->get_item(m_var_idx);
+  return m_thd->spcont->get_variable(m_var_idx);
 }
 
 const Item *
@@ -1807,7 +1807,7 @@ Item_splocal::this_item() const
 {
   DBUG_ASSERT(m_sp == m_thd->spcont->m_sp);
   DBUG_ASSERT(fixed);
-  return m_thd->spcont->get_item(m_var_idx);
+  return m_thd->spcont->get_variable(m_var_idx);
 }
 
 
@@ -1816,7 +1816,7 @@ Item_splocal::this_item_addr(THD *thd, Item **)
 {
   DBUG_ASSERT(m_sp == thd->spcont->m_sp);
   DBUG_ASSERT(fixed);
-  return thd->spcont->get_item_addr(m_var_idx);
+  return thd->spcont->get_variable_addr(m_var_idx);
 }
 
 
@@ -1913,7 +1913,7 @@ bool Item_splocal::check_cols(uint n)
 
 bool Item_splocal_row_field::fix_fields(THD *thd, Item **ref)
 {
-  Item *item= thd->spcont->get_item(m_var_idx)->element_index(m_field_idx);
+  Item *item= thd->spcont->get_variable(m_var_idx)->element_index(m_field_idx);
   return fix_fields_from_item(thd, ref, item);
 }
 
@@ -1923,7 +1923,7 @@ Item_splocal_row_field::this_item()
 {
   DBUG_ASSERT(m_sp == m_thd->spcont->m_sp);
   DBUG_ASSERT(fixed);
-  return m_thd->spcont->get_item(m_var_idx)->element_index(m_field_idx);
+  return m_thd->spcont->get_variable(m_var_idx)->element_index(m_field_idx);
 }
 
 
@@ -1932,7 +1932,7 @@ Item_splocal_row_field::this_item() const
 {
   DBUG_ASSERT(m_sp == m_thd->spcont->m_sp);
   DBUG_ASSERT(fixed);
-  return m_thd->spcont->get_item(m_var_idx)->element_index(m_field_idx);
+  return m_thd->spcont->get_variable(m_var_idx)->element_index(m_field_idx);
 }
 
 
@@ -1941,7 +1941,7 @@ Item_splocal_row_field::this_item_addr(THD *thd, Item **)
 {
   DBUG_ASSERT(m_sp == thd->spcont->m_sp);
   DBUG_ASSERT(fixed);
-  return thd->spcont->get_item(m_var_idx)->addr(m_field_idx);
+  return thd->spcont->get_variable(m_var_idx)->addr(m_field_idx);
 }
 
 
@@ -1972,7 +1972,7 @@ bool Item_splocal_row_field_by_name::fix_fields(THD *thd, Item **it)
                                                    m_var_idx,
                                                    m_field_name))
     return true;
-  Item *item= thd->spcont->get_item(m_var_idx)->element_index(m_field_idx);
+  Item *item= thd->spcont->get_variable(m_var_idx)->element_index(m_field_idx);
   set_handler(item->type_handler());
   return fix_fields_from_item(thd, it, item);
 }
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index ead04368acd..9351babdab5 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -1700,7 +1700,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount,
     /* Arguments must be fixed in Item_func_sp::fix_fields */
     DBUG_ASSERT(argp[arg_no]->fixed);
 
-    if ((err_status= nctx->set_variable(thd, arg_no, &(argp[arg_no]))))
+    if ((err_status= nctx->set_parameter(thd, arg_no, &(argp[arg_no]))))
       goto err_with_cleanup;
   }
 
@@ -1732,7 +1732,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount,
       if (arg_no)
         binlog_buf.append(',');
 
-      Item *item= nctx->get_item(arg_no);
+      Item_field *item= nctx->get_parameter(arg_no);
       str_value= item->type_handler()->print_item_value(thd, item,
                                                         &str_value_holder);
       if (str_value)
@@ -1948,7 +1948,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
         Item *tmp_item= null_item;
 
         if (!null_item ||
-            nctx->set_variable(thd, i, &tmp_item))
+            nctx->set_parameter(thd, i, &tmp_item))
         {
           DBUG_PRINT("error", ("set variable failed"));
           err_status= TRUE;
@@ -1957,7 +1957,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
       }
       else
       {
-        if (nctx->set_variable(thd, i, it_args.ref()))
+        if (nctx->set_parameter(thd, i, it_args.ref()))
         {
           DBUG_PRINT("error", ("set variable 2 failed"));
           err_status= TRUE;
@@ -2065,7 +2065,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
 
       DBUG_ASSERT(srp);
 
-      if (srp->set_value(thd, octx, nctx->get_item_addr(i)))
+      if (srp->set_value(thd, octx, nctx->get_variable_addr(i)))
       {
         DBUG_PRINT("error", ("set value failed"));
         err_status= TRUE;
@@ -2073,7 +2073,7 @@ sp_head::execute_procedure(THD *thd, List<Item> *args)
       }
 
       Send_field *out_param_info= new (thd->mem_root) Send_field();
-      nctx->get_item(i)->make_field(thd, out_param_info);
+      nctx->get_parameter(i)->make_field(thd, out_param_info);
       out_param_info->db_name= m_db.str;
       out_param_info->table_name= m_name.str;
       out_param_info->org_table_name= m_name.str;
@@ -4116,7 +4116,7 @@ sp_instr_cursor_copy_struct::exec_core(THD *thd, uint *nextp)
 {
   DBUG_ENTER("sp_instr_cursor_copy_struct::exec_core");
   int ret= 0;
-  Item_field_row *row= (Item_field_row*) thd->spcont->get_item(m_var);
+  Item_field_row *row= (Item_field_row*) thd->spcont->get_variable(m_var);
   DBUG_ASSERT(row->type_handler() == &type_handler_row);
 
   /*
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index 43d033ad83e..159a25e7594 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -317,7 +317,7 @@ bool sp_rcontext::init_var_items(THD *thd,
   uint num_vars= m_root_parsing_ctx->max_var_index();
 
   m_var_items.reset(
-    static_cast<Item **> (
+    static_cast<Item_field **> (
       thd->alloc(num_vars * sizeof (Item *))),
     num_vars);
 
@@ -634,7 +634,7 @@ int sp_rcontext::set_variable_row_field_by_name(THD *thd, uint var_idx,
 int sp_rcontext::set_variable_row(THD *thd, uint var_idx, List<Item> &items)
 {
   DBUG_ENTER("sp_rcontext::set_variable_row");
-  DBUG_ASSERT(get_item(var_idx)->cols() == items.elements);
+  DBUG_ASSERT(get_variable(var_idx)->cols() == items.elements);
   Virtual_tmp_table *vtable= virtual_tmp_table_for_row(var_idx);
   Sp_eval_expr_state state(thd);
   DBUG_RETURN(vtable->sp_set_all_fields_from_item_list(thd, items));
@@ -643,8 +643,8 @@ int sp_rcontext::set_variable_row(THD *thd, uint var_idx, List<Item> &items)
 
 Virtual_tmp_table *sp_rcontext::virtual_tmp_table_for_row(uint var_idx)
 {
-  DBUG_ASSERT(get_item(var_idx)->type() == Item::FIELD_ITEM);
-  DBUG_ASSERT(get_item(var_idx)->cmp_type() == ROW_RESULT);
+  DBUG_ASSERT(get_variable(var_idx)->type() == Item::FIELD_ITEM);
+  DBUG_ASSERT(get_variable(var_idx)->cmp_type() == ROW_RESULT);
   Field *field= m_var_table->field[var_idx];
   Virtual_tmp_table **ptable= field->virtual_tmp_table_addr();
   DBUG_ASSERT(ptable);
@@ -809,7 +809,7 @@ int sp_cursor::fetch(THD *thd, List<sp_variable> *vars, bool error_on_no_data)
   if (vars->elements != result.get_field_count() &&
       (vars->elements != 1 ||
        result.get_field_count() !=
-       thd->spcont->get_item(vars->head()->offset)->cols()))
+       thd->spcont->get_variable(vars->head()->offset)->cols()))
   {
     my_message(ER_SP_WRONG_NO_OF_FETCH_ARGS,
                ER_THD(thd, ER_SP_WRONG_NO_OF_FETCH_ARGS), MYF(0));
@@ -907,7 +907,7 @@ int sp_cursor::Select_fetch_into_spvars::send_data(List<Item> &items)
     on attempt to assign a scalar value to a ROW variable.
   */
   return spvar_list->elements == 1 &&
-         (item= thd->spcont->get_item(spvar_list->head()->offset)) &&
+         (item= thd->spcont->get_variable(spvar_list->head()->offset)) &&
          item->type_handler() == &type_handler_row &&
          item->cols() == items.elements ?
     thd->spcont->set_variable_row(thd, spvar_list->head()->offset, items) :
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index ca438107593..00816b5ea8a 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -189,6 +189,11 @@ public:
   // SP-variables.
   /////////////////////////////////////////////////////////////////////////
 
+  uint argument_count() const
+  {
+    return m_root_parsing_ctx->context_var_count();
+  }
+
   int set_variable(THD *thd, uint var_idx, Item **value);
   int set_variable_row_field(THD *thd, uint var_idx, uint field_idx,
                              Item **value);
@@ -196,11 +201,24 @@ public:
                                      const LEX_CSTRING &field_name,
                                      Item **value);
   int set_variable_row(THD *thd, uint var_idx, List<Item> &items);
-  Item *get_item(uint var_idx) const
+
+  int set_parameter(THD *thd, uint var_idx, Item **value)
+  {
+    DBUG_ASSERT(var_idx < argument_count());
+    return set_variable(thd, var_idx, value);
+  }
+
+  Item_field *get_variable(uint var_idx) const
   { return m_var_items[var_idx]; }
 
-  Item **get_item_addr(uint var_idx) const
-  { return m_var_items.array() + var_idx; }
+  Item **get_variable_addr(uint var_idx) const
+  { return ((Item **) m_var_items.array()) + var_idx; }
+
+  Item_field *get_parameter(uint var_idx) const
+  {
+    DBUG_ASSERT(var_idx < argument_count());
+    return get_variable(var_idx);
+  }
 
   bool find_row_field_by_name_or_error(uint *field_idx, uint var_idx,
                                        const LEX_CSTRING &field_name);
@@ -379,7 +397,7 @@ private:
 
   /// Collection of Item_field proxies, each of them points to the
   /// corresponding field in m_var_table.
-  Bounds_checked_array<Item *> m_var_items;
+  Bounds_checked_array<Item_field *> m_var_items;
 
   /// This is a pointer to a field, which should contain return value for
   /// stored functions (only). For stored procedures, this pointer is NULL.
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index 147f4d6eddf..0b776a2e2fe 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -3631,7 +3631,7 @@ int select_dumpvar::prepare(List<Item> &list, SELECT_LEX_UNIT *u)
       mvsp->type_handler() == &type_handler_row)
   {
     // SELECT INTO row_type_sp_variable
-    if (thd->spcont->get_item(mvsp->offset)->cols() != list.elements)
+    if (thd->spcont->get_variable(mvsp->offset)->cols() != list.elements)
       goto error;
     m_var_sp_row= mvsp;
     return 0;