Bug#35395333: Follow-up on Bug#36402968

Follow-up on Bug#36402968: renamed context to m_name_resolution_ctx
in Item_func_current_user and in Item_func_sp, initialized field in
Name_resolution_context and removed the constructor and the init method.

Change-Id: I6a86d9164f4d9bd50649032e60d2e18c217ce91c

Author: Catalin Besleaga

sql/item.h

@@ -390,7 +390,7 @@ class Item_name_string : public Name_string {
             bool is_autogenerated_arg);
 };
 
-/*
+/**
   Instances of Name_resolution_context store the information necessary for
   name resolution of Items and other context analysis of a query made in
   fix_fields().
@@ -400,57 +400,57 @@ class Item_name_string : public Name_string {
   (sql_yacc.yy)), but the structure itself will be initialized after parsing
   is complete
 
-  TODO: move subquery of INSERT ... SELECT and CREATE ... SELECT to
+  @todo move subquery of INSERT ... SELECT and CREATE ... SELECT to
   separate Query_block which allow to remove tricks of changing this
   structure before and after INSERT/CREATE and its SELECT to make correct
   field name resolution.
 */
 struct Name_resolution_context {
-  /*
+  /**
     The name resolution context to search in when an Item cannot be
     resolved in this context (the context of an outer select)
   */
-  Name_resolution_context *outer_context;
+  Name_resolution_context *outer_context{nullptr};
   /// Link to next name res context with the same query block as the base
-  Name_resolution_context *next_context;
+  Name_resolution_context *next_context{nullptr};
 
-  /*
+  /**
     List of tables used to resolve the items of this context.  Usually these
     are tables from the FROM clause of SELECT statement.  The exceptions are
     INSERT ... SELECT and CREATE ... SELECT statements, where SELECT
     subquery is not moved to a separate Query_block.  For these types of
     statements we have to change this member dynamically to ensure correct
     name resolution of different parts of the statement.
   */
-  Table_ref *table_list;
-  /*
+  Table_ref *table_list{nullptr};
+  /**
     In most cases the two table references below replace 'table_list' above
     for the purpose of name resolution. The first and last name resolution
     table references allow us to search only in a sub-tree of the nested
     join tree in a FROM clause. This is needed for NATURAL JOIN, JOIN ... USING
     and JOIN ... ON.
   */
-  Table_ref *first_name_resolution_table;
-  /*
+  Table_ref *first_name_resolution_table{nullptr};
+  /**
     Last table to search in the list of leaf table references that begins
     with first_name_resolution_table.
   */
-  Table_ref *last_name_resolution_table;
+  Table_ref *last_name_resolution_table{nullptr};
 
-  /*
+  /**
     Query_block item belong to, in case of merged VIEW it can differ from
     Query_block where item was created, so we can't use table_list/field_list
     from there
   */
-  Query_block *query_block;
+  Query_block *query_block{nullptr};
 
   /*
     Processor of errors caused during Item name resolving, now used only to
     hide underlying tables in errors about views (i.e. it substitute some
     errors for views)
   */
-  bool view_error_handler;
-  Table_ref *view_error_handler_arg;
+  bool view_error_handler{false};
+  Table_ref *view_error_handler_arg{nullptr};
 
   /**
     When true, items are resolved in this context against
@@ -460,30 +460,13 @@ struct Name_resolution_context {
 
     @see Query_block::item_list, Query_block::group_list
   */
-  bool resolve_in_select_list;
+  bool resolve_in_select_list{false};
 
-  /*
+  /**
     Security context of this name resolution context. It's used for views
     and is non-zero only if the view is defined with SQL SECURITY DEFINER.
   */
-  Security_context *security_ctx;
-
-  Name_resolution_context()
-      : outer_context(nullptr),
-        next_context(nullptr),
-        table_list(nullptr),
-        query_block(nullptr),
-        view_error_handler_arg(nullptr),
-        security_ctx(nullptr) {
-    DBUG_PRINT("outer_field", ("creating ctx %p", this));
-  }
-
-  void init() {
-    resolve_in_select_list = false;
-    view_error_handler = false;
-    first_name_resolution_table = nullptr;
-    last_name_resolution_table = nullptr;
-  }
+  Security_context *security_ctx{nullptr};
 
   void resolve_in_table_list_only(Table_ref *tables) {
     table_list = first_name_resolution_table = tables;

sql/item_func.cc

@@ -8132,7 +8132,7 @@ bool Item_func_sp::do_itemize(Parse_context *pc, Item **res) {
   THD *thd = pc->thd;
   LEX *lex = thd->lex;
 
-  context = lex->current_context();
+  m_name_resolution_ctx = lex->current_context();
   lex->safe_to_cache_query = false;
 
   if (m_name->m_db.str == nullptr) {
@@ -8223,7 +8223,8 @@ bool Item_func_sp::init_result_field(THD *thd) {
   assert(sp_result_field == nullptr);
 
   Internal_error_handler_holder<View_error_handler, Table_ref> view_handler(
-      thd, context->view_error_handler, context->view_error_handler_arg);
+      thd, m_name_resolution_ctx->view_error_handler,
+      m_name_resolution_ctx->view_error_handler_arg);
   m_sp = sp_find_routine(thd, enum_sp_type::FUNCTION, m_name,
                          &thd->sp_func_cache, true);
   if (m_sp == nullptr) {
@@ -8349,7 +8350,8 @@ bool Item_func_sp::execute() {
   THD *thd = current_thd;
 
   Internal_error_handler_holder<View_error_handler, Table_ref> view_handler(
-      thd, context->view_error_handler, context->view_error_handler_arg);
+      thd, m_name_resolution_ctx->view_error_handler,
+      m_name_resolution_ctx->view_error_handler_arg);
 
   // Bind to an instance of the stored function:
   if (m_sp == nullptr) {
@@ -8398,9 +8400,9 @@ bool Item_func_sp::execute_impl(THD *thd) {
 
   DBUG_TRACE;
 
-  if (context->security_ctx) {
+  if (m_name_resolution_ctx->security_ctx != nullptr) {
     /* Set view definer security context */
-    thd->set_security_context(context->security_ctx);
+    thd->set_security_context(m_name_resolution_ctx->security_ctx);
   }
   if (sp_check_access(thd)) goto error;
 
@@ -8505,17 +8507,18 @@ bool Item_func_sp::fix_fields(THD *thd, Item **ref) {
    */
   if (!thd->lex->is_view_context_analysis() ||
       (thd->lex->sql_command == SQLCOM_CREATE_VIEW)) {
-    if (context->security_ctx) {
+    if (m_name_resolution_ctx->security_ctx != nullptr) {
       /* Set view definer security context */
-      thd->set_security_context(context->security_ctx);
+      thd->set_security_context(m_name_resolution_ctx->security_ctx);
     }
 
     /*
       Check whether user has execute privilege or not
      */
 
     Internal_error_handler_holder<View_error_handler, Table_ref> view_handler(
-        thd, context->view_error_handler, context->view_error_handler_arg);
+        thd, m_name_resolution_ctx->view_error_handler,
+        m_name_resolution_ctx->view_error_handler_arg);
 
     const bool res = check_routine_access(thd, EXECUTE_ACL, m_name->m_db.str,
                                           m_name->m_name.str, false, false);

sql/item_func.h

@@ -3894,7 +3894,9 @@ class Item_func_sp final : public Item_func {
   typedef Item_func super;
 
  private:
-  Name_resolution_context *context{nullptr};
+  /// Holds the security definer context(if defined with SQL SECURITY DEFINER)
+  /// and the error the handler.
+  Name_resolution_context *m_name_resolution_ctx{nullptr};
   /// The name of the stored function
   sp_name *m_name{nullptr};
   /// Pointer to actual function instance (null when not resolved or executing)
@@ -3942,7 +3944,8 @@ class Item_func_sp final : public Item_func {
   bool val_json(Json_wrapper *result) override;
 
   bool change_context_processor(uchar *arg) override {
-    context = reinterpret_cast<Item_ident::Change_context *>(arg)->m_context;
+    m_name_resolution_ctx =
+        pointer_cast<Item_ident::Change_context *>(arg)->m_context;
     return false;
   }
 

sql/item_strfunc.cc

@@ -2004,32 +2004,32 @@ bool Item_func_current_user::resolve_type(THD *thd) {
   if (super::resolve_type(thd)) {
     return true;
   }
-  if (context->security_ctx == nullptr) {
+  if (m_name_resolution_ctx->security_ctx == nullptr) {
     return false;
   }
 
   // If Name_resolution_context has a definer Security_context priv_user and
   // priv_host from it are copied into the item since the
   // Name_resolution_context may have been deallocated when val_str() gets
   // called.
-  LEX_CSTRING pu = context->security_ctx->priv_user();
+  LEX_CSTRING pu = m_name_resolution_ctx->security_ctx->priv_user();
   if (pu.str != nullptr) {
-    definer_priv_user = LexStringDupRoot(thd->mem_root, pu);
-    if (definer_priv_user.str == nullptr) return true;
+    m_definer_priv_user = LexStringDupRoot(thd->mem_root, pu);
+    if (m_definer_priv_user.str == nullptr) return true;
   }
-  LEX_CSTRING ph = context->security_ctx->priv_host();
+  LEX_CSTRING ph = m_name_resolution_ctx->security_ctx->priv_host();
   if (ph.str != nullptr) {
-    definer_priv_host = LexStringDupRoot(thd->mem_root, ph);
-    if (definer_priv_host.str == nullptr) return true;
+    m_definer_priv_host = LexStringDupRoot(thd->mem_root, ph);
+    if (m_definer_priv_host.str == nullptr) return true;
   }
   return false;
 }
 
 String *Item_func_current_user::val_str(String *) {
   assert(fixed);
   if (!m_evaluated) {
-    if (definer_priv_user.str != nullptr) {
-      if (evaluate(definer_priv_user.str, definer_priv_host.str))
+    if (m_definer_priv_user.str != nullptr) {
+      if (evaluate(m_definer_priv_user.str, m_definer_priv_host.str))
         return nullptr;
     } else {
       Security_context *const ctx = current_thd->security_context();
@@ -2081,7 +2081,7 @@ bool Item_func_current_user::do_itemize(Parse_context *pc, Item **res) {
   if (skip_itemize(res)) return false;
   if (super::do_itemize(pc, res)) return true;
 
-  context = pc->thd->lex->current_context();
+  m_name_resolution_ctx = pc->thd->lex->current_context();
   return false;
 }
 

sql/item_strfunc.h

@@ -676,23 +676,23 @@ class Item_func_user : public Item_func_sysconst {
 
 class Item_func_current_user : public Item_func_user {
   typedef Item_func_user super;
-  /*
+  /**
      Used to pass a security context to the resolver functions.
      Only used for definer views. In all other contexts, the security context
      passed here is nullptr and is instead looked up dynamically at run time
      from the current THD.
   */
-  Name_resolution_context *context = nullptr;
+  Name_resolution_context *m_name_resolution_ctx = nullptr;
 
-  // Copied from context in fix_fields if definer Security_context
-  // is set in Name_resolution_context
-  LEX_CSTRING definer_priv_user = {};
-  LEX_CSTRING definer_priv_host = {};
+  /// Copied from m_name_resolution_ctx in fix_fields if the definer
+  /// Security_context is set in Name_resolution_context
+  LEX_CSTRING m_definer_priv_user = {};
+  LEX_CSTRING m_definer_priv_host = {};
 
  protected:
   type_conversion_status save_in_field_inner(Field *field, bool) override;
 
-  // Overridden to copy definer priv_user and priv_host
+  /// Overridden to copy definer priv_user and priv_host
   bool resolve_type(THD *) override;
 
  public:

sql/sql_base.cc

@@ -8520,10 +8520,12 @@ static bool test_if_string_in_list(const char *find, List<String> *str_list) {
 
 static bool set_new_item_local_context(THD *thd, Item_ident *item,
                                        Table_ref *table_ref) {
-  Name_resolution_context *context;
-  if (!(context = new (thd->mem_root) Name_resolution_context))
-    return true; /* purecov: inspected */
-  context->init();
+  Name_resolution_context *context =
+      new (thd->mem_root) Name_resolution_context;
+  if (context == nullptr) {
+    /* purecov: inspected */
+    return true;
+  }
   context->first_name_resolution_table = context->last_name_resolution_table =
       table_ref;
   context->query_block = table_ref->query_block;

sql/sql_lex.cc

@@ -2271,7 +2271,6 @@ Query_block::Query_block(MEM_ROOT *mem_root, Item *where, Item *having)
 */
 
 bool Query_block::set_context(Name_resolution_context *outer_context) {
-  context.init();
   context.query_block = this;
   context.outer_context = outer_context;
   /*

sql/sql_parse.cc

@@ -6307,9 +6307,9 @@ void Query_block::set_lock_for_tables(thr_lock_type lock_type) {
 bool push_new_name_resolution_context(Parse_context *pc, Table_ref *left_op,
                                       Table_ref *right_op) {
   THD *thd = pc->thd;
-  Name_resolution_context *on_context;
-  if (!(on_context = new (thd->mem_root) Name_resolution_context)) return true;
-  on_context->init();
+  Name_resolution_context *on_context =
+      new (thd->mem_root) Name_resolution_context;
+  if (on_context == nullptr) return true;
   on_context->first_name_resolution_table =
       left_op->first_leaf_for_name_resolution();
   on_context->last_name_resolution_table =

sql/sql_resolver.cc

@@ -6123,7 +6123,6 @@ bool Query_block::transform_grouped_to_derived(THD *thd, bool *break_off) {
     context.table_list = tl;
     context.first_name_resolution_table = tl;
     assert(context.last_name_resolution_table == nullptr);
-    new_derived->context.init();
     new_derived->context.table_list = get_table_list();
     new_derived->context.query_block = new_derived;
     new_derived->context.outer_context = &context;