Cleanups and optimization during review of new code

mysql-test/t/kill.test:
  Moved --disable_reconnect earlier to avoid race condition
sql/sql_help.cc:
  Cleanup during review of new code
  (Moved variable definitions first in function
sql/sql_insert.cc:
  Cleanup during review of new code
sql/sql_lex.cc:
  Cleanup during review of new code
sql/sql_parse.cc:
  Cleanup during review of new code
  Changed potential problem with previous_table_ref where it dependent that TABLE_LIST first element would be next_local
  Rearanged code in add_table_to_list() to remove extra if
  Combined 2 calls to calloc() to one
sql/sql_view.cc:
  Remove extra indentation level
  Combined common 'on error' exit
sql/sql_yacc.yy:
  Fixed comment style
sql/table.cc:
  Cleanup during review of new code
  - Changed while() loops to for() loop (to make code more readable)
  - Removed not needed initialization of variables
  - Removed not needed 'else' cases
  - Removed trivial ASSERT's that was checked by previous code
  - Moved comment setting last in Natural_join_column::check_grants()

mysql-test/t/kill.test

@@ -15,6 +15,7 @@ connection con1;
 drop table if exists t1;
 --enable_warnings
 
+--disable_reconnect
 create table t1 (kill_id int);
 insert into t1 values(connection_id());
 
@@ -25,7 +26,6 @@ kill @id;
 
 connection con1;
 
---disable_reconnect
 # this statement should fail
 --error 2006,2013
 select 1;

sql/sql_help.cc

@@ -623,38 +623,37 @@ bool mysqld_help(THD *thd, const char *mask)
   Protocol *protocol= thd->protocol;
   SQL_SELECT *select;
   st_find_field used_fields[array_elements(init_used_fields)];
-  DBUG_ENTER("mysqld_help");
-
   TABLE_LIST *leaves= 0;
   TABLE_LIST tables[4];
-  bzero((gptr)tables,sizeof(tables));
-  tables[0].alias= tables[0].table_name= (char*) "help_topic";
-  tables[0].lock_type= TL_READ;
-  tables[0].next_global= tables[0].next_local= &tables[1];
-  tables[0].next_name_resolution_table= tables[0].next_local;
-  tables[1].alias= tables[1].table_name= (char*) "help_category";
-  tables[1].lock_type= TL_READ;
-  tables[1].next_global= tables[1].next_local= &tables[2];
-  tables[1].next_name_resolution_table= tables[1].next_local;
-  tables[2].alias= tables[2].table_name= (char*) "help_relation";
-  tables[2].lock_type= TL_READ;
-  tables[2].next_global= tables[2].next_local= &tables[3];
-  tables[2].next_name_resolution_table= tables[2].next_local;
-  tables[3].alias= tables[3].table_name= (char*) "help_keyword";
-  tables[3].lock_type= TL_READ;
-  tables[0].db= tables[1].db= tables[2].db= tables[3].db= (char*) "mysql";
-
   List<String> topics_list, categories_list, subcategories_list;
   String name, description, example;
   int count_topics, count_categories, error;
   uint mlen= strlen(mask);
+  size_t i;
   MEM_ROOT *mem_root= thd->mem_root;
+  DBUG_ENTER("mysqld_help");
+
+  bzero((gptr)tables,sizeof(tables));
+  tables[0].alias= tables[0].table_name= (char*) "help_topic";
+  tables[0].lock_type= TL_READ;
+  tables[0].next_global= tables[0].next_local= 
+    tables[0].next_name_resolution_table= &tables[1];
+  tables[1].alias= tables[1].table_name= (char*) "help_category";
+  tables[1].lock_type= TL_READ;
+  tables[1].next_global= tables[1].next_local= 
+    tables[1].next_name_resolution_table= &tables[2];
+  tables[2].alias= tables[2].table_name= (char*) "help_relation";
+  tables[2].lock_type= TL_READ;
+  tables[2].next_global= tables[2].next_local= 
+    tables[2].next_name_resolution_table= &tables[3];
+  tables[3].alias= tables[3].table_name= (char*) "help_keyword";
+  tables[3].lock_type= TL_READ;
+  tables[0].db= tables[1].db= tables[2].db= tables[3].db= (char*) "mysql";
 
   if (open_and_lock_tables(thd, tables))
     goto error;
   /*
     Init tables and fields to be usable from items
-
     tables do not contain VIEWs => we can pass 0 as conds
   */
   setup_tables(thd, &thd->lex->select_lex.context,
@@ -663,7 +662,6 @@ bool mysqld_help(THD *thd, const char *mask)
   memcpy((char*) used_fields, (char*) init_used_fields, sizeof(used_fields));
   if (init_fields(thd, tables, used_fields, array_elements(used_fields)))
     goto error;
-  size_t i;
   for (i=0; i<sizeof(tables)/sizeof(TABLE_LIST); i++)
     tables[i].table->file->init_table_handle_for_HANDLER();
 

sql/sql_insert.cc

@@ -891,6 +891,7 @@ 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;
+      /* first_name_resolution_table was set by resolve_in_table_list_only() */
       context->first_name_resolution_table->
         next_name_resolution_table=          save_next_local;
     }
@@ -2199,6 +2200,7 @@ 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;
+      /* first_name_resolution_table was set by resolve_in_table_list_only() */
       context->first_name_resolution_table->
         next_name_resolution_table=          save_next_local;
     }

sql/sql_lex.cc

@@ -1867,7 +1867,7 @@ TABLE_LIST *st_lex::unlink_first_table(bool *link_to_local)
     */
     if ((*link_to_local= test(select_lex.table_list.first)))
     {
-      select_lex.context.table_list= first->next_local;
+      select_lex.context.table_list= 
         select_lex.context.first_name_resolution_table= first->next_local;
       select_lex.table_list.first= (byte*) (first->next_local);
       select_lex.table_list.elements--;	//safety

sql/sql_parse.cc

@@ -3258,7 +3258,7 @@ end_with_restore_list:
       /* Skip first table, which is the table we are inserting in */
       TABLE_LIST *second_table= first_table->next_local;
       select_lex->table_list.first= (byte*) second_table;
-      select_lex->context.table_list= second_table;
+      select_lex->context.table_list= 
         select_lex->context.first_name_resolution_table= second_table;
       res= mysql_insert_select_prepare(thd);
       if (!res && (result= new select_insert(first_table, first_table->table,
@@ -3274,8 +3274,8 @@ end_with_restore_list:
           which in turn resets context.table_list and
           context.first_name_resolution_table.
         */
-        select_lex->context.table_list= first_table->next_local;
+        select_lex->context.table_list= 
-        select_lex->context.first_name_resolution_table= first_table->next_local;
+          select_lex->context.first_name_resolution_table= second_table;
 	res= handle_select(thd, lex, result, OPTION_SETUP_TABLES_DONE);
         delete result;
       }
@@ -6016,6 +6016,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
   char *alias_str;
   LEX *lex= thd->lex;
   DBUG_ENTER("add_table_to_list");
+  LINT_INIT(previous_table_ref);
 
   if (!table)
     DBUG_RETURN(0);				// End of memory
@@ -6111,9 +6112,23 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
   /* Store the table reference preceding the current one. */
   if (table_list.elements > 0)
   {
-    previous_table_ref= (TABLE_LIST*) table_list.next;
+    /*
+      table_list.next points to the last inserted TABLE_LIST->next_local'
+      element
+    */
+    previous_table_ref= (TABLE_LIST*) (table_list.next -
+                                       offsetof(TABLE_LIST, next_local));
     DBUG_ASSERT(previous_table_ref);
+    /*
+      Set next_name_resolution_table of the previous table reference to point
+      to the current table reference. In effect the list
+      TABLE_LIST::next_name_resolution_table coincides with
+      TABLE_LIST::next_local. Later this may be changed in
+      store_top_level_join_columns() for NATURAL/USING joins.
+    */
+    previous_table_ref->next_name_resolution_table= ptr;
   }
+
   /*
     Link the current table reference in a local list (list for current select).
     Notice that as a side effect here we set the next_local field of the
@@ -6121,15 +6136,6 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
     list 'table_list'.
   */
   table_list.link_in_list((byte*) ptr, (byte**) &ptr->next_local);
-  /*
-    Set next_name_resolution_table of the previous table reference to point to
-    the current table reference. In effect the list
-    TABLE_LIST::next_name_resolution_table coincides with
-    TABLE_LIST::next_local. Later this may be changed in
-    store_top_level_join_columns() for NATURAL/USING joins.
-   */
-  if (table_list.elements > 1)
-    previous_table_ref->next_name_resolution_table= ptr;
   ptr->next_name_resolution_table= NULL;
   /* Link table in global list (all used tables) */
   lex->add_to_query_tables(ptr);
@@ -6164,10 +6170,12 @@ bool st_select_lex::init_nested_join(THD *thd)
   NESTED_JOIN *nested_join;
   DBUG_ENTER("init_nested_join");
 
-  if (!(ptr = (TABLE_LIST *) thd->calloc(sizeof(TABLE_LIST))) ||
+  if (!(ptr= (TABLE_LIST*) thd->calloc(ALIGN_SIZE(sizeof(TABLE_LIST))+
-      !(nested_join= ptr->nested_join=
+                                       sizeof(NESTED_JOIN))))
-                    (NESTED_JOIN *) thd->calloc(sizeof(NESTED_JOIN))))
     DBUG_RETURN(1);
+  nested_join= ptr->nested_join=
+    ((NESTED_JOIN*) ((byte*) ptr + ALIGN_SIZE(sizeof(TABLE_LIST))));
+
   join_list->push_front(ptr);
   ptr->embedding= embedding;
   ptr->join_list= join_list;
@@ -6235,25 +6243,30 @@ TABLE_LIST *st_select_lex::end_nested_join(THD *thd)
     The function nest last join operation as if it was enclosed in braces.
 
   RETURN VALUE
-    Pointer to TABLE_LIST element created for the new nested join, if success
+    0  Error
-    0, otherwise
+    #  Pointer to TABLE_LIST element created for the new nested join
+
 */
 
 TABLE_LIST *st_select_lex::nest_last_join(THD *thd)
 {
   TABLE_LIST *ptr;
   NESTED_JOIN *nested_join;
+  List<TABLE_LIST> *embedded_list;
   DBUG_ENTER("nest_last_join");
 
-  if (!(ptr = (TABLE_LIST *) thd->calloc(sizeof(TABLE_LIST))) ||
+  if (!(ptr= (TABLE_LIST*) thd->calloc(ALIGN_SIZE(sizeof(TABLE_LIST))+
-      !(nested_join= ptr->nested_join=
+                                       sizeof(NESTED_JOIN))))
-                    (NESTED_JOIN *) thd->calloc(sizeof(NESTED_JOIN))))
     DBUG_RETURN(0);
+  nested_join= ptr->nested_join=
+    ((NESTED_JOIN*) ((byte*) ptr + ALIGN_SIZE(sizeof(TABLE_LIST))));
+
   ptr->embedding= embedding;
   ptr->join_list= join_list;
-  List<TABLE_LIST> *embedded_list= &nested_join->join_list;
+  embedded_list= &nested_join->join_list;
   embedded_list->empty();
-  for (int i=0; i < 2; i++)
+
+  for (uint i=0; i < 2; i++)
   {
     TABLE_LIST *table= join_list->pop();
     table->join_list= embedded_list;

sql/sql_view.cc

@@ -120,12 +120,11 @@ static void make_unique_view_field_name(Item *target,
 
 bool check_duplicate_names(List<Item> &item_list, bool gen_unique_view_name)
 {
-  DBUG_ENTER("check_duplicate_names");
-  /* Test absence of duplicates names */
-  {
   Item *item;
   List_iterator_fast<Item> it(item_list);
   List_iterator_fast<Item> itc(item_list);
+  DBUG_ENTER("check_duplicate_names");
+
   while ((item= it++))
   {
     Item *check;
@@ -138,24 +137,21 @@ bool check_duplicate_names(List<Item> &item_list, bool gen_unique_view_name)
       if (my_strcasecmp(system_charset_info, item->name, check->name) == 0)
       {
         if (!gen_unique_view_name)
-          {
+          goto err;
-            my_error(ER_DUP_FIELDNAME, MYF(0), item->name);
+        if (item->is_autogenerated_name)
-            DBUG_RETURN(TRUE);
-          }
-          else if (item->is_autogenerated_name)
           make_unique_view_field_name(item, item_list, item);
         else if (check->is_autogenerated_name)
           make_unique_view_field_name(check, item_list, item);
         else
-          {
+          goto err;
-            my_error(ER_DUP_FIELDNAME, MYF(0), item->name);
-            DBUG_RETURN(TRUE);
-          }
-        }
       }
     }
   }
   DBUG_RETURN(FALSE);
+
+err:
+  my_error(ER_DUP_FIELDNAME, MYF(0), item->name);
+  DBUG_RETURN(TRUE);
 }
 
 

sql/sql_yacc.yy

@@ -8681,8 +8681,10 @@ union_list:
 	}
 	select_init
         {
-          /* Remove from the name resolution context stack the context of the
+          /*
-             last select in the union. */
+	    Remove from the name resolution context stack the context of the
+            last select in the union.
+	  */
           Lex->pop_context();
         }
 	;

sql/table.cc

@@ -2186,21 +2186,24 @@ bool st_table_list::is_leaf_for_name_resolution()
     reverse order.
 
   RETURN
-    - If 'this' is a nested table reference - the left-most child of
+    If 'this' is a nested table reference - the left-most child of
       the tree rooted in 'this',
-    - else - 'this'
+    else return 'this'
 */
 
 TABLE_LIST *st_table_list::first_leaf_for_name_resolution()
 {
-  TABLE_LIST *cur_table_ref= this;
+  TABLE_LIST *cur_table_ref;
-  TABLE_LIST *next;
+  NESTED_JOIN *cur_nested_join;
-  NESTED_JOIN *cur_nested_join= nested_join;
+  LINT_INIT(cur_table_ref);
 
-  if (this->is_leaf_for_name_resolution())
+  if (is_leaf_for_name_resolution())
     return this;
+  DBUG_ASSERT(nested_join);
 
-  while (cur_nested_join)
+  for (cur_nested_join= nested_join;
+       cur_nested_join;
+       cur_nested_join= cur_table_ref->nested_join)
   {
     List_iterator_fast<TABLE_LIST> it(cur_nested_join->join_list);
     cur_table_ref= it++;
@@ -2210,12 +2213,12 @@ TABLE_LIST *st_table_list::first_leaf_for_name_resolution()
     */
     if (!(cur_table_ref->outer_join & JOIN_TYPE_RIGHT))
     {
+      TABLE_LIST *next;
       while ((next= it++))
         cur_table_ref= next;
     }
     if (cur_table_ref->is_leaf_for_name_resolution())
       break;
-    cur_nested_join= cur_table_ref->nested_join;
   }
   return cur_table_ref;
 }
@@ -2247,13 +2250,16 @@ TABLE_LIST *st_table_list::first_leaf_for_name_resolution()
 TABLE_LIST *st_table_list::last_leaf_for_name_resolution()
 {
   TABLE_LIST *cur_table_ref= this;
-  TABLE_LIST *next;
+  NESTED_JOIN *cur_nested_join;
-  NESTED_JOIN *cur_nested_join= nested_join;
+  LINT_INIT(cur_table_ref);
 
-  if (this->is_leaf_for_name_resolution())
+  if (is_leaf_for_name_resolution())
     return this;
+  DBUG_ASSERT(nested_join);
 
-  while (cur_nested_join)
+  for (cur_nested_join= nested_join;
+       cur_nested_join;
+       cur_nested_join= cur_table_ref->nested_join)
   {
     /*
       If 'this' is a RIGHT JOIN, the operands in 'join_list' are in reverse
@@ -2262,6 +2268,7 @@ TABLE_LIST *st_table_list::last_leaf_for_name_resolution()
     if ((cur_table_ref->outer_join & JOIN_TYPE_RIGHT))
     {
       List_iterator_fast<TABLE_LIST> it(cur_nested_join->join_list);
+      TABLE_LIST *next;
       cur_table_ref= it++;
       while ((next= it++))
         cur_table_ref= next;
@@ -2270,7 +2277,6 @@ TABLE_LIST *st_table_list::last_leaf_for_name_resolution()
       cur_table_ref= cur_nested_join->join_list.head();
     if (cur_table_ref->is_leaf_for_name_resolution())
       break;
-    cur_nested_join= cur_table_ref->nested_join;
   }
   return cur_table_ref;
 }
@@ -2306,11 +2312,8 @@ const char *Natural_join_column::name()
     DBUG_ASSERT(table_field == NULL);
     return view_field->name;
   }
-  else
+
-  {
-    DBUG_ASSERT(view_field == NULL);
   return table_field->field_name;
-  }
 }
 
 
@@ -2319,13 +2322,10 @@ Item *Natural_join_column::create_item(THD *thd)
   if (view_field)
   {
     DBUG_ASSERT(table_field == NULL);
-    return create_view_field(thd, table_ref, &view_field->item, view_field->name);
+    return create_view_field(thd, table_ref, &view_field->item,
+                             view_field->name);
   }
-  else
-  {
-    DBUG_ASSERT(view_field == NULL);
   return new Item_field(thd, &thd->lex->current_select->context, table_field);
-  }
 }
 
 
@@ -2336,31 +2336,29 @@ Field *Natural_join_column::field()
     DBUG_ASSERT(table_field == NULL);
     return NULL;
   }
-  else
-  {
-    DBUG_ASSERT(view_field == NULL);
   return table_field;
-  }
 }
 
 
 const char *Natural_join_column::table_name()
 {
   return table_ref->alias;
-/*
+  /*
-  TODO: I think that it is sufficient to return just
+    TODO:
+    I think that it is sufficient to return just
     table->alias, which is correctly set to either
     the view name, the table name, or the alias to
     the table reference (view or stored table).
+  */
+#ifdef NOT_YET
   if (view_field)
     return table_ref->view_name.str;
-  else
+
-  {
   DBUG_ASSERT(!strcmp(table_ref->table_name,
                       table_ref->table->s->table_name));
   return table_ref->table_name;
-  }
+}
-*/
+#endif
 }
 
 
@@ -2368,12 +2366,10 @@ const char *Natural_join_column::db_name()
 {
   if (view_field)
     return table_ref->view_db.str;
-  else
+
-  {
   DBUG_ASSERT(!strcmp(table_ref->db,
                       table_ref->table->s->db));
   return table_ref->db;
-  }
 }
 
 
@@ -2381,7 +2377,6 @@ GRANT_INFO *Natural_join_column::grant()
 {
   if (view_field)
     return &(table_ref->grant);
-  else
   return &(table_ref->table->grant);
 }
 
@@ -2402,16 +2397,17 @@ GRANT_INFO *Natural_join_column::grant()
     it is a view or a stored table colum.
 
   RETURN
-    FALSE - if the column can be accessed
+    FALSE  The column can be accessed
-    TRUE - if there are no access rights to all equivalent columns
+    TRUE   There are no access rights to all equivalent columns
 */
 
 bool
 Natural_join_column::check_grants(THD *thd, const char *name, uint length)
 {
-  GRANT_INFO *grant= NULL; /* If NULL do not check access rights. */
+  GRANT_INFO *grant;
   const char *db_name;
   const char *table_name;
+
   if (view_field)
   {
     DBUG_ASSERT(table_field == NULL);
@@ -2544,7 +2540,6 @@ void Field_iterator_table_ref::set_field_iterator()
                  (!table_ref->field_translation &&
                   table_ref->join_columns->elements ==
                   table_ref->table->s->fields)));
-    natural_join_it.set(table_ref);
     field_it= &natural_join_it;
     DBUG_PRINT("info",("field_it for '%s' is Field_iterator_natural_join",
                        table_ref->table_name));
@@ -2554,7 +2549,6 @@ void Field_iterator_table_ref::set_field_iterator()
   {
     DBUG_ASSERT(table_ref->view &&
                 table_ref->effective_algorithm == VIEW_ALGORITHM_MERGE);
-    view_field_it.set(table_ref);
     field_it= &view_field_it;
     DBUG_PRINT("info", ("field_it for '%s' is Field_iterator_view",
                         table_ref->table_name));
@@ -2563,12 +2557,11 @@ void Field_iterator_table_ref::set_field_iterator()
   else
   {
     DBUG_ASSERT(table_ref->table || table_ref->view);
-    table_field_it.set(table_ref);
     field_it= &table_field_it;
     DBUG_PRINT("info", ("field_it for '%s' is Field_iterator_table",
                         table_ref->table_name));
-
   }
+  field_it->set(table_ref);
   DBUG_VOID_RETURN;
 }
 
@@ -2607,12 +2600,10 @@ const char *Field_iterator_table_ref::table_name()
     return table_ref->view_name.str;
   else if (table_ref->is_natural_join)
     return natural_join_it.column_ref()->table_name();
-  else
+
-  {
   DBUG_ASSERT(!strcmp(table_ref->table_name,
                       table_ref->table->s->table_name));
   return table_ref->table_name;
-  }
 }
 
 
@@ -2622,11 +2613,9 @@ const char *Field_iterator_table_ref::db_name()
     return table_ref->view_db.str;
   else if (table_ref->is_natural_join)
     return natural_join_it.column_ref()->db_name();
-  else
+
-  {
   DBUG_ASSERT(!strcmp(table_ref->db, table_ref->table->s->db));
   return table_ref->db;
-  }
 }
 
 
@@ -2636,7 +2625,6 @@ GRANT_INFO *Field_iterator_table_ref::grant()
     return &(table_ref->grant);
   else if (table_ref->is_natural_join)
     return natural_join_it.column_ref()->grant();
-  else
   return &(table_ref->table->grant);
 }
 
@@ -2645,7 +2633,6 @@ bool Field_iterator_table_ref::is_coalesced()
 {
   if (table_ref->is_natural_join)
     return natural_join_it.column_ref()->is_coalesced;
-  else
   return FALSE;
 }
 
@@ -2663,29 +2650,30 @@ bool Field_iterator_table_ref::is_coalesced()
     TODO
 
   RETURN
-    Pointer to a column of a natural join (or its operand)
+    #     Pointer to a column of a natural join (or its operand)
-    NULL if there was no memory to allocate the column
+    NULL  No memory to allocate the column
 */
 
 Natural_join_column *
 Field_iterator_table_ref::get_or_create_column_ref(THD *thd, bool *is_created)
 {
+  Natural_join_column *nj_col;
+
   *is_created= TRUE;
   if (field_it == &table_field_it)
     return new Natural_join_column(table_field_it.field(), table_ref);
-  else if (field_it == &view_field_it)
+  if (field_it == &view_field_it)
-    return new Natural_join_column(view_field_it.field_translator(), table_ref);
+    return new Natural_join_column(view_field_it.field_translator(),
-  else
+                                   table_ref);
-  {
+
   /*
     This is NATURAL join, we already have created a column reference,
     so just return it.
   */
   *is_created= FALSE;
-    Natural_join_column *nj_col= natural_join_it.column_ref();
+  nj_col= natural_join_it.column_ref();
   DBUG_ASSERT(nj_col);
   return nj_col;
-  }
 }