A cleanup for MDEV-13919 sql_mode=ORACLE: Derive length of VARCHAR SP param...

The intent of this patch is to avoid copying arguments from
a pair "Item **args, uint arg_count" to List<Item> in
sp_head::execute_function(). If the number of a stored function parameters
is huge, such copying can affect performance.

Change:
1. Adding a new method Row_definition_list::adjust_formal_params_to_actual_params,
   which accepts a pair of "Item **, uint".
2. Modifying the code to use the new method:
   - the calls for sp_rcontext::retrieve_field_definitions() and
     Row_definition_list::adjust_formal_params_to_actual_params() have
     been moved from sp_rcontext::create() to sp_head::rcontext_create(),
     to handle different argument notations easier (Item** vs List<Item>).
   - sp_rcontext::create() now assumes that the passed Row_definition_list
     is already adjusted to the actual SP parameters, and all "TYPE OF"
     and "ROWTYPE OF" references are resolved.
3. Removing creation of List<Item> in sp_head::execute_procedure(),
   using the code with "Item**, uint" notation instead.
4. Improvement of the code for MDEV-10577:
   As a good side effect, this patch gets rid of double security context
   switch inside sp_head::execute_trigger():
   sp_rcontext is created when the context is already switched,
   so the second context switch inside sp_head::rcontext_create() was
   redundant. This is solved by adding a "bool switch_secutiry_ctx" parameter
   to rcontext_create(), so now execute_function() and execute_procedure()
   pass "true", while execute_trigger() passes "false".

5 files changed, 62 insertions, 28 deletions

diff --git a/sql/field.h b/sql/field.h
index 32ff75042e0..548445eb1fa 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -4096,6 +4096,8 @@ public:
     return 0;
   }
   bool adjust_formal_params_to_actual_params(THD *thd, List<Item> *args);
+  bool adjust_formal_params_to_actual_params(THD *thd,
+                                             Item **args, uint arg_count);
   bool resolve_type_refs(THD *);
 };
 
diff --git a/sql/sp_head.cc b/sql/sp_head.cc
index 16688a4ae63..c797d172e5d 100644
--- a/sql/sp_head.cc
+++ b/sql/sp_head.cc
@@ -1427,7 +1427,7 @@ bool sp_head::check_execute_access(THD *thd) const
 
 
 /**
-  Create rcontext using the routine security.
+  Create rcontext optionally using the routine security.
   This is important for sql_mode=ORACLE to make sure that the invoker has
   access to the tables mentioned in the %TYPE references.
 
@@ -1440,25 +1440,51 @@ bool sp_head::check_execute_access(THD *thd) const
   @retval          !NULL - success (the invoker has rights to all %TYPE tables)
 */
 sp_rcontext *sp_head::rcontext_create(THD *thd, Field *ret_value,
-                                      List<Item> *args)
+                                      Row_definition_list *defs,
+                                      bool switch_security_ctx)
 {
-  bool has_column_type_refs= m_flags & HAS_COLUMN_TYPE_REFS;
+  if (!(m_flags & HAS_COLUMN_TYPE_REFS))
+    return sp_rcontext::create(thd, m_pcont, ret_value, *defs);
+  sp_rcontext *res= NULL;
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
   Security_context *save_security_ctx;
-  if (has_column_type_refs &&
+  if (switch_security_ctx &&
       set_routine_security_ctx(thd, this, &save_security_ctx))
     return NULL;
 #endif
-  sp_rcontext *res= sp_rcontext::create(thd, m_pcont, ret_value,
-                                        has_column_type_refs, args);
+  if (!defs->resolve_type_refs(thd))
+    res= sp_rcontext::create(thd, m_pcont, ret_value, *defs);
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-  if (has_column_type_refs)
+  if (switch_security_ctx)
     m_security_ctx.restore_security_context(thd, save_security_ctx);
 #endif
   return res;
 }
 
 
+sp_rcontext *sp_head::rcontext_create(THD *thd, Field *ret_value,
+                                      List<Item> *args)
+{
+  DBUG_ASSERT(args);
+  Row_definition_list defs;
+  m_pcont->retrieve_field_definitions(&defs);
+  if (defs.adjust_formal_params_to_actual_params(thd, args))
+    return NULL;
+  return rcontext_create(thd, ret_value, &defs, true);
+}
+
+
+sp_rcontext *sp_head::rcontext_create(THD *thd, Field *ret_value,
+                                      Item **args, uint arg_count)
+{
+  Row_definition_list defs;
+  m_pcont->retrieve_field_definitions(&defs);
+  if (defs.adjust_formal_params_to_actual_params(thd, args, arg_count))
+    return NULL;
+  return rcontext_create(thd, ret_value, &defs, true);
+}
+
+
 /**
   Execute trigger stored program.
 
@@ -1556,8 +1582,9 @@ sp_head::execute_trigger(THD *thd,
   init_sql_alloc(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0, MYF(0));
   thd->set_n_backup_active_arena(&call_arena, &backup_arena);
 
-  if (!(nctx= sp_rcontext::create(thd, m_pcont, NULL,
-                                  m_flags & HAS_COLUMN_TYPE_REFS, NULL)))
+  Row_definition_list defs;
+  m_pcont->retrieve_field_definitions(&defs);
+  if (!(nctx= rcontext_create(thd, NULL, &defs, false)))
   {
     err_status= TRUE;
     goto err_with_cleanup;
@@ -1638,7 +1665,6 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount,
   MEM_ROOT call_mem_root;
   Query_arena call_arena(&call_mem_root, Query_arena::STMT_INITIALIZED_FOR_SP);
   Query_arena backup_arena;
-  List<Item> largs;
   DBUG_ENTER("sp_head::execute_function");
   DBUG_PRINT("info", ("function %s", m_name.str));
 
@@ -1673,10 +1699,7 @@ sp_head::execute_function(THD *thd, Item **argp, uint argcount,
   init_sql_alloc(&call_mem_root, MEM_ROOT_BLOCK_SIZE, 0, MYF(0));
   thd->set_n_backup_active_arena(&call_arena, &backup_arena);
 
-  for (uint i= 0 ; i < argcount ; i++)
-    largs.push_back(argp[i]);
-
-  if (!(nctx= rcontext_create(thd, return_value_fld, &largs)))
+  if (!(nctx= rcontext_create(thd, return_value_fld, argp, argcount)))
   {
     thd->restore_active_arena(&call_arena, &backup_arena);
     err_status= TRUE;
diff --git a/sql/sp_head.h b/sql/sp_head.h
index 2f4f21a76f4..272bc5d445c 100644
--- a/sql/sp_head.h
+++ b/sql/sp_head.h
@@ -216,7 +216,11 @@ public:
   }
 
   sp_rcontext *rcontext_create(THD *thd, Field *retval, List<Item> *args);
-
+  sp_rcontext *rcontext_create(THD *thd, Field *retval,
+                               Item **args, uint arg_count);
+  sp_rcontext *rcontext_create(THD *thd, Field *retval,
+                               Row_definition_list *list,
+                               bool switch_security_ctx);
 private:
   /**
     Version of the stored routine cache at the moment when the
diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc
index e82947835f0..43a42b579bd 100644
--- a/sql/sp_rcontext.cc
+++ b/sql/sp_rcontext.cc
@@ -63,25 +63,15 @@ sp_rcontext::~sp_rcontext()
 sp_rcontext *sp_rcontext::create(THD *thd,
                                  const sp_pcontext *root_parsing_ctx,
                                  Field *return_value_fld,
-                                 bool resolve_type_refs,
-                                 List<Item> *args)
+                                 Row_definition_list &field_def_lst)
 {
   sp_rcontext *ctx= new (thd->mem_root) sp_rcontext(root_parsing_ctx,
                                                     return_value_fld,
                                                     thd->in_sub_stmt);
-
   if (!ctx)
     return NULL;
 
-  Row_definition_list field_def_lst;
-  ctx->m_root_parsing_ctx->retrieve_field_definitions(&field_def_lst);
-
-  if (args &&
-      field_def_lst.adjust_formal_params_to_actual_params(thd, args))
-    return NULL;
-
   if (ctx->alloc_arrays(thd) ||
-      (resolve_type_refs && field_def_lst.resolve_type_refs(thd)) ||
       ctx->init_var_table(thd, field_def_lst) ||
       ctx->init_var_items(thd, field_def_lst))
   {
@@ -110,6 +100,22 @@ bool Row_definition_list::
 }
 
 
+bool Row_definition_list::
+       adjust_formal_params_to_actual_params(THD *thd,
+                                             Item **args, uint arg_count)
+{
+  List_iterator<Spvar_definition> it(*this);
+  DBUG_ASSERT(elements >= arg_count );
+  Spvar_definition *def;
+  for (uint i= 0; (def= it++) && (i < arg_count) ; i++)
+  {
+    if (def->type_handler()->adjust_spparam_type(def, args[i]))
+      return true;
+  }
+  return false;
+}
+
+
 bool sp_rcontext::alloc_arrays(THD *thd)
 {
   {
diff --git a/sql/sp_rcontext.h b/sql/sp_rcontext.h
index 2638aad03bf..93c8cacc70a 100644
--- a/sql/sp_rcontext.h
+++ b/sql/sp_rcontext.h
@@ -71,8 +71,7 @@ public:
   static sp_rcontext *create(THD *thd,
                              const sp_pcontext *root_parsing_ctx,
                              Field *return_value_fld,
-                             bool resolve_type_refs,
-                             List<Item> *args);
+                             Row_definition_list &defs);
 
   ~sp_rcontext();