WL#2486 - Natural/using join according to SQL:2003.

Post-review fixes according to Monty's review.


sql/item.h:
  Unite all code that stores and restores the state of a name resolution context
  into a class to represent the state, and methods to save/restore that
  state.
sql/mysql_priv.h:
  Reorder parameters so that length is after the name of a field,
  and database is before table name.
sql/sql_acl.cc:
  Reorder parameters so that length is after the name of a field,
  and database is before table name.
sql/sql_base.cc:
  * Reorder parameters so that length is after the name of a field,
    and database is before table name.
  * Added new method - Field_iterator_table_ref::get_natural_column_ref
    to avoid unnecessary code when it is knwon that no new columns will
    be created when accessing natural join columns.
sql/sql_insert.cc:
  Unite all code that stores and restores the state of a name resolution context
  into a class to represent the state, and methods to save/restore that
  state.
sql/sql_lex.cc:
  Removed obsolete comment.
sql/sql_lex.h:
  Return error from push_contex() if there is no memory.
sql/sql_list.h:
  Extended base_list_iterator, List_iterator, and List_iterator_fast with an
  empty constructor, and init() methods, so that one doesn't have to construct
  a new iterator object every time one needs to iterate over a new list.
sql/sql_parse.cc:
  Moved common functionality from the parser into one function, and renamed
  the function to better reflect what it does.
sql/sql_yacc.yy:
  Moved common functionality from the parser into one function, and renamed
  the function to better reflect what it does.
sql/table.cc:
  * Extended base_list_iterator, List_iterator, and List_iterator_fast with an
    empty constructor, and init() methods, so that one doesn't have to construct
    a new iterator object every time one needs to iterate over a new list.
  * Added new method Field_iterator_table_ref::get_natural_column_ref to be
    used in cases when it is known for sure that no new columns should be
    created.
sql/table.h:
  - column_ref_it no longer allocated for each new list of columns
  - new method get_natural_join_column for faster/simpler access
    to natural join columns.

sql/item.h

@@ -326,6 +326,48 @@ struct Name_resolution_context: Sql_alloc
 };
 
 
+/*
+  Store and restore the current state of a name resolution context.
+*/
+
+class Name_resolution_context_state
+{
+private:
+  TABLE_LIST *save_table_list;
+  TABLE_LIST *save_first_name_resolution_table;
+  TABLE_LIST *save_next_name_resolution_table;
+  bool        save_resolve_in_select_list;
+
+public:
+  TABLE_LIST *save_next_local;
+
+public:
+  /* Save the state of a name resolution context. */
+  void save_state(Name_resolution_context *context, TABLE_LIST *table_list)
+  {
+    save_table_list=                  context->table_list;
+    save_first_name_resolution_table= context->first_name_resolution_table;
+    save_next_name_resolution_table=  (context->first_name_resolution_table) ?
+                                      context->first_name_resolution_table->
+                                               next_name_resolution_table :
+                                      NULL;
+    save_resolve_in_select_list=      context->resolve_in_select_list;
+    save_next_local=                  table_list->next_local;
+  }
+
+  /* Restore a name resolution context from saved state. */
+  void restore_state(Name_resolution_context *context, TABLE_LIST *table_list)
+  {
+    table_list->next_local=                save_next_local;
+    context->table_list=                   save_table_list;
+    context->first_name_resolution_table=  save_first_name_resolution_table;
+    if (context->first_name_resolution_table)
+      context->first_name_resolution_table->
+               next_name_resolution_table= save_next_name_resolution_table;
+    context->resolve_in_select_list=       save_resolve_in_select_list;
+  }
+};
+
 /*************************************************************************/
 
 typedef bool (Item::*Item_processor)(byte *arg);

sql/mysql_priv.h

@@ -791,12 +791,11 @@ find_field_in_tables(THD *thd, Item_ident *item,
                      bool check_privileges, bool register_tree_change);
 Field *
 find_field_in_table_ref(THD *thd, TABLE_LIST *table_list,
-                        const char *name, const char *item_name,
-                        const char *table_name, const char *db_name,
-                        uint length, Item **ref,
+                        const char *name, uint length,
+                        const char *item_name, const char *db_name,
+                        const char *table_name, Item **ref,
                         bool check_grants_table, bool check_grants_view,
-                        bool allow_rowid,
-                        uint *cached_field_index_ptr,
+                        bool allow_rowid, uint *cached_field_index_ptr,
                         bool register_tree_change, TABLE_LIST **actual_table);
 Field *
 find_field_in_table(THD *thd, TABLE *table, const char *name,
@@ -918,7 +917,8 @@ create_field * new_create_field(THD *thd, char *field_name, enum_field_types typ
 				uint uint_geom_type);
 void store_position_for_column(const char *name);
 bool add_to_list(THD *thd, SQL_LIST &list,Item *group,bool asc);
-Name_resolution_context *make_join_on_context(THD *thd, TABLE_LIST *left_op,
+bool push_new_name_resolution_context(THD *thd,
+                                      TABLE_LIST *left_op,
                                       TABLE_LIST *right_op);
 void add_join_on(TABLE_LIST *b,Item *expr);
 void add_join_natural(TABLE_LIST *a,TABLE_LIST *b,List<String> *using_fields);

sql/sql_acl.cc

@@ -2761,8 +2761,9 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list,
         uint unused_field_idx= NO_CACHED_FIELD_INDEX;
         TABLE_LIST *dummy;
         Field *f=find_field_in_table_ref(thd, table_list, column->column.ptr(),
+                                         column->column.length(),
                                          column->column.ptr(), NULL, NULL,
-                                         column->column.length(), 0, 1, 1, 0,
+                                         0, 1, 1, 0,
                                          &unused_field_idx, FALSE, &dummy);
         if (f == (Field*)0)
         {

sql/sql_base.cc

@@ -2732,8 +2732,8 @@ static bool check_grant_column_in_sctx(THD *thd, GRANT_INFO *grant,
     thd				thread handler
     table_list			view to search for 'name'
     name			name of field
-    item_name                   name of item if it will be created (VIEW)
     length			length of name
+    item_name                   name of item if it will be created (VIEW)
     ref				expression substituted in VIEW should be passed
                                 using this reference (return view_ref_found)
     check_grants		do check columns grants for view?
@@ -2748,9 +2748,9 @@ static bool check_grant_column_in_sctx(THD *thd, GRANT_INFO *grant,
 
 static Field *
 find_field_in_view(THD *thd, TABLE_LIST *table_list,
-                   const char *name, const char *item_name,
-                   uint length, Item **ref, bool check_grants,
-                   bool register_tree_change)
+                   const char *name, uint length,
+                   const char *item_name, Item **ref,
+                   bool check_grants, bool register_tree_change)
 {
   DBUG_ENTER("find_field_in_view");
   DBUG_PRINT("enter",
@@ -2766,13 +2766,6 @@ find_field_in_view(THD *thd, TABLE_LIST *table_list,
   {
     if (!my_strcasecmp(system_charset_info, field_it.name(), name))
     {
-      if (table_list->schema_table_reformed)
-        /*
-          Translation table items are always Item_fields and fixed already
-          ('mysql_schema_table' function). So we can return ->field. It is
-          used only for 'show & where' commands.
-        */
-        DBUG_RETURN(((Item_field*) (field_it.item()))->field);
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
       if (check_grant_column_in_sctx(thd, &table_list->grant,
                                      table_list->view_db.str,
@@ -2784,6 +2777,10 @@ find_field_in_view(THD *thd, TABLE_LIST *table_list,
       // in PS use own arena or data will be freed after prepare
       if (register_tree_change)
         arena= thd->activate_stmt_arena_if_needed(&backup);
+      /*
+        create_item() may, or may not create a new Item, depending on
+        the column reference. See create_view_field() for details.
+      */
       Item *item= field_it.create_item(thd);
       if (register_tree_change && arena)
         thd->restore_active_arena(arena, &backup);
@@ -2880,15 +2877,13 @@ find_field_in_natural_join(THD *thd, TABLE_LIST *table_ref, const char *name,
   if (nj_col->view_field)
   {
     Item *item;
-    /*
-      The found field is a view field, we do as in find_field_in_view()
-      and return a pointer to pointer to the Item of that field.
-    */
     if (register_tree_change)
       arena= thd->activate_stmt_arena_if_needed(&backup);
-
+    /*
+      create_item() may, or may not create a new Item, depending on the
+      column reference. See create_view_field() for details.
+    */
     item= nj_col->create_item(thd);
-
     if (register_tree_change && arena)
       thd->restore_active_arena(arena, &backup);
 
@@ -3006,10 +3001,10 @@ find_field_in_table(THD *thd, TABLE *table, const char *name, uint length,
     thd			   [in]  thread handler
     table_list		   [in]  table reference to search
     name		   [in]  name of field
+    length		   [in]  field length of name
     item_name              [in]  name of item if it will be created (VIEW)
-    table_name             [in]  optional table name that qualifies the field
     db_name                [in]  optional database name that qualifies the
-    length		   [in]  field length of name
+    table_name             [in]  optional table name that qualifies the field
     ref		       [in/out] if 'name' is resolved to a view field, ref
                                  is set to point to the found view field
     check_grants_table	   [in]  do check columns grants for table?
@@ -3043,9 +3038,9 @@ find_field_in_table(THD *thd, TABLE *table, const char *name, uint length,
 
 Field *
 find_field_in_table_ref(THD *thd, TABLE_LIST *table_list,
-                        const char *name, const char *item_name,
-                        const char *table_name, const char *db_name,
-                        uint length, Item **ref,
+                        const char *name, uint length,
+                        const char *item_name, const char *db_name,
+                        const char *table_name, Item **ref,
                         bool check_grants_table, bool check_grants_view,
                         bool allow_rowid, uint *cached_field_index_ptr,
                         bool register_tree_change, TABLE_LIST **actual_table)
@@ -3092,7 +3087,7 @@ find_field_in_table_ref(THD *thd, TABLE_LIST *table_list,
   if (table_list->field_translation)
   {
     /* 'table_list' is a view or an information schema table. */
-    if ((fld= find_field_in_view(thd, table_list, name, item_name, length,
+    if ((fld= find_field_in_view(thd, table_list, name, length, item_name,
                                  ref, check_grants_view,
                                  register_tree_change)))
       *actual_table= table_list;
@@ -3132,8 +3127,8 @@ find_field_in_table_ref(THD *thd, TABLE_LIST *table_list,
       TABLE_LIST *table;
       while ((table= it++))
       {
-        if ((fld= find_field_in_table_ref(thd, table, name, item_name,
-                                          table_name, db_name, length, ref,
+        if ((fld= find_field_in_table_ref(thd, table, name, length, item_name,
+                                          db_name, table_name, ref,
                                           check_grants_table,
                                           check_grants_view,
                                           allow_rowid, cached_field_index_ptr,
@@ -3241,8 +3236,8 @@ find_field_in_tables(THD *thd, Item_ident *item,
                                  1, &(item->cached_field_index),
                                  table_ref->security_ctx);
     else
-      found= find_field_in_table_ref(thd, table_ref, name, item->name,
-                                     NULL, NULL, length, ref,
+      found= find_field_in_table_ref(thd, table_ref, name, length, item->name,
+                                     NULL, NULL, ref,
                                      (table_ref->table &&
                                       test(table_ref->table->grant.
                                            want_privilege) &&
@@ -3289,9 +3284,8 @@ find_field_in_tables(THD *thd, Item_ident *item,
   for (; cur_table != last_table ;
        cur_table= cur_table->next_name_resolution_table)
   {
-    Field *cur_field= find_field_in_table_ref(thd, cur_table, name, item->name,
-                                              table_name, db,
-                                              length, ref,
+    Field *cur_field= find_field_in_table_ref(thd, cur_table, name, length,
+                                              item->name, db, table_name, ref,
                                               (cur_table->table &&
                                                test(cur_table->table->grant.
                                                     want_privilege) &&
@@ -3707,7 +3701,7 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2,
   {
     bool is_created_1;
     bool found= FALSE;
-    if (!(nj_col_1= it_1.get_or_create_column_ref(thd, &is_created_1)))
+    if (!(nj_col_1= it_1.get_or_create_column_ref(&is_created_1)))
       goto err;
     field_name_1= nj_col_1->name();
 
@@ -3728,7 +3722,7 @@ mark_common_columns(THD *thd, TABLE_LIST *table_ref_1, TABLE_LIST *table_ref_2,
       bool is_created_2;
       Natural_join_column *cur_nj_col_2;
       const char *cur_field_name_2;
-      if (!(cur_nj_col_2= it_2.get_or_create_column_ref(thd, &is_created_2)))
+      if (!(cur_nj_col_2= it_2.get_or_create_column_ref(&is_created_2)))
         goto err;
       cur_field_name_2= cur_nj_col_2->name();
 
@@ -3920,13 +3914,7 @@ store_natural_using_join_columns(THD *thd, TABLE_LIST *natural_using_join,
   /* Append the columns of the first join operand. */
   for (it_1.set(table_ref_1); !it_1.end_of_fields(); it_1.next())
   {
-    if (!(nj_col_1= it_1.get_or_create_column_ref(thd, &is_created)))
-      goto err;
-    /*
-      The following assert checks that mark_common_columns() was run and
-      we created the list table_ref_1->join_columns.
-    */
-    DBUG_ASSERT(!is_created);
+    nj_col_1= it_1.get_natural_column_ref();
     if (nj_col_1->is_common)
     {
       natural_using_join->join_columns->push_back(nj_col_1);
@@ -3972,13 +3960,7 @@ store_natural_using_join_columns(THD *thd, TABLE_LIST *natural_using_join,
   /* Append the non-equi-join columns of the second join operand. */
   for (it_2.set(table_ref_2); !it_2.end_of_fields(); it_2.next())
   {
-    if (!(nj_col_2= it_2.get_or_create_column_ref(thd, &is_created)))
-      goto err;
-    /*
-      The following assert checks that mark_common_columns() was run and
-      we created the list table_ref_2->join_columns.
-    */
-    DBUG_ASSERT(!is_created);
+    nj_col_2= it_2.get_natural_column_ref();
     if (!nj_col_2->is_common)
       non_join_columns->push_back(nj_col_2);
     else
@@ -4712,8 +4694,7 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name,
             because it was already created and stored with the natural join.
           */
           Natural_join_column *nj_col;
-          if (!(nj_col= field_iterator.get_or_create_column_ref(thd,
-                                                                &is_created)))
+          if (!(nj_col= field_iterator.get_or_create_column_ref(&is_created)))
             DBUG_RETURN(TRUE);
           DBUG_ASSERT(nj_col->table_field && !is_created);
           field_table= nj_col->table_ref->table;

sql/sql_insert.cc

@@ -108,11 +108,7 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
   {						// Part field list
     SELECT_LEX *select_lex= &thd->lex->select_lex;
     Name_resolution_context *context= &select_lex->context;
-    TABLE_LIST *save_next_local;
-    TABLE_LIST *save_table_list;
-    TABLE_LIST *save_first_name_resolution_table;
-    TABLE_LIST *save_next_name_resolution_table;
-    bool        save_resolve_in_select_list;
+    Name_resolution_context_state ctx_state;
     int res;
 
     if (fields.elements != values.elements)
@@ -125,14 +121,7 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
     select_lex->no_wrap_view_item= TRUE;
 
     /* Save the state of the current name resolution context. */
-    save_table_list=                  context->table_list;
-    save_first_name_resolution_table= context->first_name_resolution_table;
-    save_next_name_resolution_table=  (context->first_name_resolution_table) ?
-                                      context->first_name_resolution_table->
-                                               next_name_resolution_table :
-                                      NULL;
-    save_resolve_in_select_list=      context->resolve_in_select_list;
-    save_next_local=                  table_list->next_local;
+    ctx_state.save_state(context, table_list);
 
     /*
       Perform name resolution only in the first table - 'table_list',
@@ -143,13 +132,7 @@ static int check_insert_fields(THD *thd, TABLE_LIST *table_list,
     res= setup_fields(thd, 0, fields, 1, 0, 0);
 
     /* Restore the current context. */
-    table_list->next_local=                save_next_local;
-    context->table_list=                   save_table_list;
-    context->first_name_resolution_table=  save_first_name_resolution_table;
-    if (context->first_name_resolution_table)
-      context->first_name_resolution_table->
-               next_name_resolution_table= save_next_name_resolution_table;
-    context->resolve_in_select_list=       save_resolve_in_select_list;
+    ctx_state.restore_state(context, table_list);
     thd->lex->select_lex.no_wrap_view_item= FALSE;
 
     if (res)
@@ -280,13 +263,10 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
   ulonglong id;
   COPY_INFO info;
   TABLE *table= 0;
-  TABLE_LIST *save_table_list;
-  TABLE_LIST *save_next_local;
-  TABLE_LIST *save_first_name_resolution_table;
-  TABLE_LIST *save_next_name_resolution_table;
   List_iterator_fast<List_item> its(values_list);
   List_item *values;
   Name_resolution_context *context;
+  Name_resolution_context_state ctx_state;
 #ifndef EMBEDDED_LIBRARY
   char *query= thd->query;
 #endif
@@ -367,13 +347,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
 
   context= &thd->lex->select_lex.context;
   /* Save the state of the current name resolution context. */
-  save_table_list=                  context->table_list;
-  save_first_name_resolution_table= context->first_name_resolution_table;
-  save_next_name_resolution_table=  (context->first_name_resolution_table) ?
-                                    context->first_name_resolution_table->
-                                             next_name_resolution_table :
-                                    NULL;
-  save_next_local=                  table_list->next_local;
+  ctx_state.save_state(context, table_list);
 
   /*
     Perform name resolution only in the first table - 'table_list',
@@ -397,16 +371,11 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
   its.rewind ();
  
   /* Restore the current context. */
-  table_list->next_local= save_next_local;
-  context->first_name_resolution_table= save_first_name_resolution_table;
-  if (context->first_name_resolution_table)
-    context->first_name_resolution_table->
-             next_name_resolution_table= save_next_name_resolution_table;
+  ctx_state.restore_state(context, table_list);
 
   /*
     Fill in the given fields and dump it to the table file
   */
-
   info.records= info.deleted= info.copied= info.updated= 0;
   info.ignore= ignore;
   info.handle_duplicates=duplic;
@@ -814,11 +783,7 @@ bool mysql_prepare_insert(THD *thd, TABLE_LIST *table_list,
 {
   SELECT_LEX *select_lex= &thd->lex->select_lex;
   Name_resolution_context *context= &select_lex->context;
-  TABLE_LIST *save_table_list;
-  TABLE_LIST *save_next_local;
-  TABLE_LIST *save_first_name_resolution_table;
-  TABLE_LIST *save_next_name_resolution_table;
-  bool        save_resolve_in_select_list;
+  Name_resolution_context_state ctx_state;
   bool insert_into_view= (table_list->view != 0);
   bool res= 0;
   DBUG_ENTER("mysql_prepare_insert");
@@ -858,15 +823,7 @@ bool mysql_prepare_insert(THD *thd, TABLE_LIST *table_list,
     DBUG_RETURN(TRUE);
 
   /* Save the state of the current name resolution context. */
-  save_table_list=                  context->table_list;
-  /* Here first_name_resolution_table points to the first select table. */
-  save_first_name_resolution_table= context->first_name_resolution_table;
-  save_next_name_resolution_table=  (context->first_name_resolution_table) ?
-                                    context->first_name_resolution_table->
-                                             next_name_resolution_table :
-                                    NULL;
-  save_resolve_in_select_list=      context->resolve_in_select_list;
-  save_next_local=                  table_list->next_local;
+  ctx_state.save_state(context, table_list);
 
   /*
     Perform name resolution only in the first table - 'table_list',
@@ -891,23 +848,17 @@ bool mysql_prepare_insert(THD *thd, TABLE_LIST *table_list,
     */       
     if (select_lex->group_list.elements == 0)
     {
-      context->table_list->next_local=       save_next_local;
+      context->table_list->next_local=       ctx_state.save_next_local;
       /* first_name_resolution_table was set by resolve_in_table_list_only() */
       context->first_name_resolution_table->
-        next_name_resolution_table=          save_next_local;
+        next_name_resolution_table=          ctx_state.save_next_local;
     }
     if (!res)
       res= setup_fields(thd, 0, update_values, 1, 0, 0);
   }
 
   /* Restore the current context. */
-  table_list->next_local= save_next_local;
-  context->table_list= save_table_list;
-  context->first_name_resolution_table= save_first_name_resolution_table;
-  if (context->first_name_resolution_table)
-    context->first_name_resolution_table->
-             next_name_resolution_table= save_next_name_resolution_table;
-  context->resolve_in_select_list= save_resolve_in_select_list;
+  ctx_state.restore_state(context, table_list);
 
   if (res)
     DBUG_RETURN(res);
@@ -2176,17 +2127,10 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
   {
     /* Save the state of the current name resolution context. */
     Name_resolution_context *context= &lex->select_lex.context;
-    TABLE_LIST *save_table_list;
-    TABLE_LIST *save_next_local;
-    TABLE_LIST *save_first_name_resolution_table;
-    TABLE_LIST *save_next_name_resolution_table;
-    save_table_list=                  context->table_list;
-    save_first_name_resolution_table= context->first_name_resolution_table;
-    save_next_name_resolution_table=  (context->first_name_resolution_table) ?
-                                      context->first_name_resolution_table->
-                                               next_name_resolution_table :
-                                      NULL;
-    save_next_local= table_list->next_local;
+    Name_resolution_context_state ctx_state;
+
+    /* Save the state of the current name resolution context. */
+    ctx_state.save_state(context, table_list);
 
     /* Perform name resolution only in the first table - 'table_list'. */
     table_list->next_local= 0;
@@ -2202,20 +2146,15 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
     */       
     if (lex->select_lex.group_list.elements == 0)
     {
-      context->table_list->next_local=       save_next_local;
+      context->table_list->next_local=       ctx_state.save_next_local;
       /* first_name_resolution_table was set by resolve_in_table_list_only() */
       context->first_name_resolution_table->
-        next_name_resolution_table=          save_next_local;
+        next_name_resolution_table=          ctx_state.save_next_local;
     }
     res= res || setup_fields(thd, 0, *info.update_values, 1, 0, 0);
 
     /* Restore the current context. */
-    table_list->next_local= save_next_local;
-    context->first_name_resolution_table= save_first_name_resolution_table;
-    if (context->first_name_resolution_table)
-      context->first_name_resolution_table->
-               next_name_resolution_table= save_next_name_resolution_table;
-
+    ctx_state.restore_state(context, table_list);
   }
 
   lex->current_select= lex_current_select_save;

sql/sql_lex.cc

@@ -1127,6 +1127,11 @@ void st_select_lex::init_query()
   /*
     Add the name resolution context of the current (sub)query to the
     stack of contexts for the whole query.
+    TODO:
+    push_context may return an error if there is no memory for a new
+    element in the stack, however this method has no return value,
+    thus push_context should be moved to a place where query
+    initialization is checked for failure.
   */
   parent_lex->push_context(&context);
   cond_count= with_wild= 0;

sql/sql_lex.h

@@ -1006,9 +1006,9 @@ typedef struct st_lex
   }
   void cleanup_after_one_table_open();
 
-  void push_context(Name_resolution_context *context)
+  bool push_context(Name_resolution_context *context)
   {
-    context_stack.push_front(context);
+    return context_stack.push_front(context);
   }
 
   void pop_context()

sql/sql_list.h

@@ -266,10 +266,21 @@ protected:
     ls.elements= elm;
   }
 public:
-  base_list_iterator(base_list &list_par) 
-    :list(&list_par), el(&list_par.first), prev(0), current(0)
+  base_list_iterator() 
+    :list(0), el(0), prev(0), current(0)
   {}
 
+  base_list_iterator(base_list &list_par) 
+  { init(list_par); }
+
+  inline void init(base_list &list_par)
+  {
+    list= &list_par;
+    el= &list_par.first;
+    prev= 0;
+    current= 0;
+  }
+
   inline void *next(void)
   {
     prev=el;
@@ -364,6 +375,8 @@ template <class T> class List_iterator :public base_list_iterator
 {
 public:
   List_iterator(List<T> &a) : base_list_iterator(a) {}
+  List_iterator() : base_list_iterator() {}
+  inline void init(List<T> &a) { base_list_iterator::init(a); }
   inline T* operator++(int) { return (T*) base_list_iterator::next(); }
   inline T *replace(T *a)   { return (T*) base_list_iterator::replace(a); }
   inline T *replace(List<T> &a) { return (T*) base_list_iterator::replace(a); }
@@ -385,6 +398,8 @@ protected:
 
 public:
   inline List_iterator_fast(List<T> &a) : base_list_iterator(a) {}
+  inline List_iterator_fast() : base_list_iterator() {}
+  inline void init(List<T> &a) { base_list_iterator::init(a); }
   inline T* operator++(int) { return (T*) base_list_iterator::next_fast(); }
   inline void rewind(void)  { base_list_iterator::rewind(); }
   void sublist(List<T> &list_arg, uint el_arg)

sql/sql_parse.cc

@@ -6584,36 +6584,39 @@ void st_select_lex::set_lock_for_tables(thr_lock_type lock_type)
 
 
 /*
-  Create a new name resolution context for a JOIN ... ON clause.
+  Push a new name resolution context for a JOIN ... ON clause to the
+  context stack of a query block.
 
   SYNOPSIS
-    make_join_on_context()
+    push_new_name_resolution_context()
     thd       pointer to current thread
     left_op   left  operand of the JOIN
     right_op  rigth operand of the JOIN
 
   DESCRIPTION
     Create a new name resolution context for a JOIN ... ON clause,
-    and set the first and last leaves of the list of table references
-    to be used for name resolution.
+    set the first and last leaves of the list of table references
+    to be used for name resolution, and push the newly created
+    context to the stack of contexts of the query.
 
   RETURN
-    A new context if all is OK
-    NULL - if a memory allocation error occured
+    FALSE  if all is OK
+    TRUE   if a memory allocation error occured
 */
 
-Name_resolution_context *
-make_join_on_context(THD *thd, TABLE_LIST *left_op, TABLE_LIST *right_op)
+bool
+push_new_name_resolution_context(THD *thd,
+                                 TABLE_LIST *left_op, TABLE_LIST *right_op)
 {
   Name_resolution_context *on_context;
   if (!(on_context= new (thd->mem_root) Name_resolution_context))
-    return NULL;
+    return TRUE;
   on_context->init();
   on_context->first_name_resolution_table=
     left_op->first_leaf_for_name_resolution();
   on_context->last_name_resolution_table=
     right_op->last_leaf_for_name_resolution();
-  return on_context;
+  return thd->lex->push_context(on_context);
 }
 
 

sql/sql_yacc.yy

@@ -5175,10 +5175,8 @@ join_table:
           {
             YYERROR_UNLESS($1 && ($$=$3));
             /* Change the current name resolution context to a local context. */
-            Name_resolution_context *on_context;
-            if (!(on_context= make_join_on_context(YYTHD,$1,$3)))
+            if (push_new_name_resolution_context(YYTHD, $1, $3))
               YYABORT;
-            Lex->push_context(on_context);
           }
           expr
 	  {
@@ -5190,10 +5188,8 @@ join_table:
           {
             YYERROR_UNLESS($1 && ($$=$3));
             /* Change the current name resolution context to a local context. */
-            Name_resolution_context *on_context;
-            if (!(on_context= make_join_on_context(YYTHD,$1,$3)))
+            if (push_new_name_resolution_context(YYTHD, $1, $3))
               YYABORT;
-            Lex->push_context(on_context);
           }
           expr
           {
@@ -5220,10 +5216,8 @@ join_table:
           ON
           {
             /* Change the current name resolution context to a local context. */
-            Name_resolution_context *on_context;
-            if (!(on_context= make_join_on_context(YYTHD,$1,$5)))
+            if (push_new_name_resolution_context(YYTHD, $1, $5))
               YYABORT;
-            Lex->push_context(on_context);
           }
           expr
 	  {
@@ -5253,10 +5247,8 @@ join_table:
           ON
           {
             /* Change the current name resolution context to a local context. */
-            Name_resolution_context *on_context;
-            if (!(on_context= make_join_on_context(YYTHD,$1,$5)))
+            if (push_new_name_resolution_context(YYTHD, $1, $5))
               YYABORT;
-            Lex->push_context(on_context);
           }
           expr
           {
@@ -5317,10 +5309,9 @@ table_factor:
           ON
           {
             /* Change the current name resolution context to a local context. */
-            Name_resolution_context *on_context;
-            if (!(on_context= make_join_on_context(YYTHD,$3,$7)))
+            if (push_new_name_resolution_context(YYTHD, $3, $7))
               YYABORT;
-            Lex->push_context(on_context);
+
           }
           expr '}'
 	  {

sql/table.cc

@@ -2702,8 +2702,9 @@ Item *create_view_field(THD *thd, TABLE_LIST *view, Item **field_ref,
   if (view->schema_table_reformed)
   {
     /*
-      In case of SHOW command (schema_table_reformed set) all items are
-      fixed
+      Translation table items are always Item_fields and already fixed
+      ('mysql_schema_table' function). So we can return directly the
+      field. This case happens only for 'show & where' commands.
     */
     DBUG_ASSERT(field && field->fixed);
     DBUG_RETURN(field);
@@ -2735,21 +2736,14 @@ Item *create_view_field(THD *thd, TABLE_LIST *view, Item **field_ref,
 void Field_iterator_natural_join::set(TABLE_LIST *table_ref)
 {
   DBUG_ASSERT(table_ref->join_columns);
-  delete column_ref_it;
-
-  /*
-    TODO: try not to allocate new iterator every time. If we have to,
-    then check for out of memory condition.
-  */
-  column_ref_it= new List_iterator_fast<Natural_join_column>
-                     (*(table_ref->join_columns));
-  cur_column_ref= (*column_ref_it)++;
+  column_ref_it.init(*(table_ref->join_columns));
+  cur_column_ref= column_ref_it++;
 }
 
 
 void Field_iterator_natural_join::next()
 {
-  cur_column_ref= (*column_ref_it)++;
+  cur_column_ref= column_ref_it++;
   DBUG_ASSERT(!cur_column_ref || ! cur_column_ref->table_field ||
               cur_column_ref->table_ref->table ==
               cur_column_ref->table_field->table);
@@ -2876,7 +2870,6 @@ GRANT_INFO *Field_iterator_table_ref::grant()
 
   SYNOPSIS
     Field_iterator_table_ref::get_or_create_column_ref()
-    thd         [in]  pointer to current thread
     is_created  [out] set to TRUE if the column was created,
                       FALSE if we return an already created colum
 
@@ -2889,7 +2882,7 @@ GRANT_INFO *Field_iterator_table_ref::grant()
 */
 
 Natural_join_column *
-Field_iterator_table_ref::get_or_create_column_ref(THD *thd, bool *is_created)
+Field_iterator_table_ref::get_or_create_column_ref(bool *is_created)
 {
   Natural_join_column *nj_col;
 
@@ -2923,6 +2916,41 @@ Field_iterator_table_ref::get_or_create_column_ref(THD *thd, bool *is_created)
 }
 
 
+/*
+  Return an existing reference to a column of a natural/using join.
+
+  SYNOPSIS
+    Field_iterator_table_ref::get_natural_column_ref()
+
+  DESCRIPTION
+    The method should be called in contexts where it is expected that
+    all natural join columns are already created, and that the column
+    being retrieved is a Natural_join_column.
+
+  RETURN
+    #     Pointer to a column of a natural join (or its operand)
+    NULL  No memory to allocate the column
+*/
+
+Natural_join_column *
+Field_iterator_table_ref::get_natural_column_ref()
+{
+  Natural_join_column *nj_col;
+
+  DBUG_ASSERT(field_it == &natural_join_it);
+  /*
+    The field belongs to a NATURAL join, therefore the column reference was
+    already created via one of the two constructor calls above. In this case
+    we just return the already created column reference.
+  */
+  nj_col= natural_join_it.column_ref();
+  DBUG_ASSERT(nj_col &&
+              (!nj_col->table_field ||
+               nj_col->table_ref->table == nj_col->table_field->table));
+  return nj_col;
+}
+
+
 /*****************************************************************************
 ** Instansiate templates
 *****************************************************************************/

sql/table.h

@@ -734,11 +734,11 @@ public:
 
 class Field_iterator_natural_join: public Field_iterator
 {
-  List_iterator_fast<Natural_join_column> *column_ref_it;
+  List_iterator_fast<Natural_join_column> column_ref_it;
   Natural_join_column *cur_column_ref;
 public:
-  Field_iterator_natural_join() :column_ref_it(NULL), cur_column_ref(NULL) {}
-  ~Field_iterator_natural_join() { delete column_ref_it; }
+  Field_iterator_natural_join() :cur_column_ref(NULL) {}
+  ~Field_iterator_natural_join() {}
   void set(TABLE_LIST *table);
   void next();
   bool end_of_fields() { return !cur_column_ref; }
@@ -785,7 +785,8 @@ public:
   GRANT_INFO *grant();
   Item *create_item(THD *thd) { return field_it->create_item(thd); }
   Field *field() { return field_it->field(); }
-  Natural_join_column *get_or_create_column_ref(THD *thd, bool *is_created);
+  Natural_join_column *get_or_create_column_ref(bool *is_created);
+  Natural_join_column *get_natural_column_ref();
 };