Optimizer code cleanups, no logic changes

- Updated comments
- Added some extra DEBUG
- Indentation changes and break long lines
- Trivial code changes like:
  - Combining 2 statements in one
  - Reorder DBUG lines
  - Use a variable to store a pointer that is used multiple times
- Moved declaration of variables to start of loop/function
- Removed dead or commented code
- Removed wrong DBUG_EXECUTE code in best_extension_by_limited_search()

sql/filesort.cc

@@ -1544,7 +1544,7 @@ static bool check_if_pq_applicable(Sort_param *param,
     DBUG_RETURN(false);
   }
 
-  if (param->max_rows + 2 >= UINT_MAX)
+  if (param->max_rows >= UINT_MAX - 2)
   {
     DBUG_PRINT("info", ("Too large LIMIT"));
     DBUG_RETURN(false);
@@ -2171,8 +2171,8 @@ Type_handler_timestamp_common::sort_length(THD *thd,
 
 void
 Type_handler_int_result::sort_length(THD *thd,
-                                        const Type_std_attributes *item,
-                                        SORT_FIELD_ATTR *sortorder) const
+                                     const Type_std_attributes *item,
+                                     SORT_FIELD_ATTR *sortorder) const
 {
   sortorder->original_length= sortorder->length= 8; // Sizof intern longlong
 }
@@ -2180,8 +2180,8 @@ Type_handler_int_result::sort_length(THD *thd,
 
 void
 Type_handler_real_result::sort_length(THD *thd,
-                                        const Type_std_attributes *item,
-                                        SORT_FIELD_ATTR *sortorder) const
+                                      const Type_std_attributes *item,
+                                      SORT_FIELD_ATTR *sortorder) const
 {
   sortorder->original_length= sortorder->length= sizeof(double);
 }
@@ -2205,8 +2205,8 @@ Type_handler_decimal_result::sort_length(THD *thd,
   @param thd			  Thread handler
   @param sortorder		  Order of items to sort
   @param s_length	          Number of items to sort
-  @param allow_packing_for_sortkeys [out]  set to false if packing sort keys is not
-                                     allowed
+  @param allow_packing_for_sortkeys [out]  set to false if packing sort keys
+                                  is not allowed
 
   @note
    * sortorder->length and other members are updated for each sort item.

sql/multi_range_read.cc

@@ -37,8 +37,8 @@
   @param n_ranges_arg    Number of ranges in the sequence, or 0 if the caller
                          can't efficiently determine it
   @param bufsz    INOUT  IN:  Size of the buffer available for use
-                         OUT: Size of the buffer that is expected to be actually
-                              used, or 0 if buffer is not needed.
+                         OUT: Size of the buffer that is expected to be
+                              actually used, or 0 if buffer is not needed.
   @param flags    INOUT  A combination of HA_MRR_* flags
   @param cost     OUT    Estimated cost of MRR access
 
@@ -286,11 +286,15 @@ handler::multi_range_read_info_const(uint keyno, RANGE_SEQ_IF *seq,
     (single_point_ranges - assigned_single_point_ranges).
 
     We don't add these to io_blocks as we don't want to penalize equal
-    readss (if we did, a range that would read 5 rows would be
+    reads (if we did, a range that would read 5 rows would be
     regarded as better than one equal read).
 
     Better to assume we have done a records_in_range() for the equal
     range and it's also cached.
+
+    One effect of this is that io_blocks for simple ranges are often 0,
+    as the blocks where already read by records_in_range and we assume
+    that we don't have to read it again.
   */
   io_blocks= (range_blocks_cnt - edge_blocks_cnt);
   unassigned_single_point_ranges+= (single_point_ranges -
@@ -1991,9 +1995,10 @@ bool DsMrr_impl::get_disk_sweep_mrr_cost(uint keynr, ha_rows rows, uint flags,
   else
   {
     cost->reset();
-    *buffer_size= (uint)MY_MAX(*buffer_size, 
-                      (size_t)(1.2*rows_in_last_step) * elem_size + 
-                      primary_file->ref_length + table->key_info[keynr].key_length);
+    *buffer_size= ((uint) MY_MAX(*buffer_size,
+                                 (size_t)(1.2*rows_in_last_step) * elem_size +
+                                 primary_file->ref_length +
+                                 table->key_info[keynr].key_length));
   }
   
   Cost_estimate last_step_cost;

sql/opt_subselect.cc

@@ -2507,24 +2507,13 @@ bool optimize_semijoin_nests(JOIN *join, table_map all_table_map)
         sjm->is_used= FALSE;
         double subjoin_out_rows, subjoin_read_time;
 
-        /*
-        join->get_partial_cost_and_fanout(n_tables + join->const_tables,
-                                          table_map(-1),
-                                          &subjoin_read_time, 
-                                          &subjoin_out_rows);
-        */
-        join->get_prefix_cost_and_fanout(n_tables, 
+        join->get_prefix_cost_and_fanout(n_tables,
                                          &subjoin_read_time,
                                          &subjoin_out_rows);
 
         sjm->materialization_cost.convert_from_cost(subjoin_read_time);
         sjm->rows_with_duplicates= sjm->rows= subjoin_out_rows;
         
-        // Don't use the following list because it has "stale" items. use
-        // ref_pointer_array instead:
-        //
-        //List<Item> &right_expr_list= 
-        //  sj_nest->sj_subq_pred->unit->first_select()->item_list;
         /*
           Adjust output cardinality estimates. If the subquery has form
 
@@ -3432,8 +3421,8 @@ bool Firstmatch_picker::check_qep(JOIN *join,
             optimizer_flag(join->thd, OPTIMIZER_SWITCH_SEMIJOIN_WITH_CACHE))
         {
           /* 
-            An important special case: only one inner table, and @@optimizer_switch
-            allows join buffering.
+            An important special case: only one inner table, and
+            @@optimizer_switch allows join buffering.
              - read_time is the same (i.e. FirstMatch doesn't add any cost
              - remove fanout added by the last table
           */
@@ -3583,8 +3572,7 @@ bool Duplicate_weedout_picker::check_qep(JOIN *join,
       Add the cost of temptable use. The table will have sj_outer_fanout
       records, and we will make 
       - sj_outer_fanout table writes
-      - sj_inner_fanout*sj_outer_fanout  lookups.
-
+      - sj_inner_fanout*sj_outer_fanout lookups.
     */
     double one_lookup_cost= get_tmp_table_lookup_cost(join->thd,
                                                       sj_outer_fanout,
@@ -3657,33 +3645,37 @@ void JOIN::dbug_verify_sj_inner_tables(uint prefix_size) const
 */
 
 void restore_prev_sj_state(const table_map remaining_tables, 
-                                  const JOIN_TAB *tab, uint idx)
+                           const JOIN_TAB *tab, uint idx)
 {
   TABLE_LIST *emb_sj_nest;
 
-  if (tab->emb_sj_nest)
+  if ((emb_sj_nest= tab->emb_sj_nest))
   {
-    table_map subq_tables= tab->emb_sj_nest->sj_inner_tables;
+    table_map subq_tables= emb_sj_nest->sj_inner_tables;
     tab->join->sjm_lookup_tables &= ~subq_tables;
-  }
 
-  if (!tab->join->emb_sjm_nest && (emb_sj_nest= tab->emb_sj_nest))
-  {
-    table_map subq_tables= emb_sj_nest->sj_inner_tables &
-                           ~tab->join->const_table_map;
-    /* If we're removing the last SJ-inner table, remove the sj-nest */
-    if ((remaining_tables & subq_tables) == subq_tables)
+    if (!tab->join->emb_sjm_nest)
     {
-      // All non-const tables of the SJ nest are in the remaining_tables.
-      // we are not in the nest anymore.
-      tab->join->cur_sj_inner_tables &= ~emb_sj_nest->sj_inner_tables;
-    }
-    else
-    {
-      // Semi-join nest has:
-      // - a table being removed (not in the prefix)
-      // - some tables in the prefix.
-      tab->join->cur_sj_inner_tables |= emb_sj_nest->sj_inner_tables;
+      table_map subq_tables= (emb_sj_nest->sj_inner_tables &
+                              ~tab->join->const_table_map);
+      /* If we're removing the last SJ-inner table, remove the sj-nest */
+      if ((remaining_tables & subq_tables) == subq_tables)
+      {
+        /*
+          All non-const tables of the SJ nest are in the remaining_tables.
+          we are not in the nest anymore.
+        */
+        tab->join->cur_sj_inner_tables &= ~emb_sj_nest->sj_inner_tables;
+      }
+      else
+      {
+        /*
+          Semi-join nest has:
+          - a table being removed (not in the prefix)
+          - some tables in the prefix.
+        */
+        tab->join->cur_sj_inner_tables |= emb_sj_nest->sj_inner_tables;
+      }
     }
   }
 
@@ -6634,7 +6626,6 @@ bool JOIN::choose_subquery_plan(table_map join_tables)
 
       /* Get the cost of the modified IN-EXISTS plan. */
       inner_read_time_2= inner_join->best_read;
-
     }
     else
     {

sql/opt_subselect.h

@@ -320,7 +320,7 @@ void optimize_semi_joins(JOIN *join, table_map remaining_tables, uint idx,
 void update_sj_state(JOIN *join, const JOIN_TAB *new_tab,
                      uint idx, table_map remaining_tables);
 void restore_prev_sj_state(const table_map remaining_tables, 
-                                  const JOIN_TAB *tab, uint idx);
+                           const JOIN_TAB *tab, uint idx);
 
 void fix_semijoin_strategies_for_picked_join_order(JOIN *join);
 

sql/opt_trace.cc

@@ -568,35 +568,32 @@ void Opt_trace_stmt::set_allowed_mem_size(size_t mem_size)
 
 void Json_writer::add_table_name(const JOIN_TAB *tab)
 {
+  char table_name_buffer[SAFE_NAME_LEN];
+  DBUG_ASSERT(tab != NULL);
   DBUG_ASSERT(tab->join->thd->trace_started());
-  if (tab != NULL)
+
+  if (tab->table && tab->table->derived_select_number)
   {
-    char table_name_buffer[SAFE_NAME_LEN];
-    if (tab->table && tab->table->derived_select_number)
-    {
-      /* Derived table name generation */
-      size_t len= my_snprintf(table_name_buffer, sizeof(table_name_buffer)-1,
-                             "<derived%u>",
-                             tab->table->derived_select_number);
-      add_str(table_name_buffer, len);
-    }
-    else if (tab->bush_children)
-    {
-      JOIN_TAB *ctab= tab->bush_children->start;
-      size_t len= my_snprintf(table_name_buffer,
-                           sizeof(table_name_buffer)-1,
-                           "<subquery%d>",
-                           ctab->emb_sj_nest->sj_subq_pred->get_identifier());
-      add_str(table_name_buffer, len);
-    }
-    else
-    {
-      TABLE_LIST *real_table= tab->table->pos_in_table_list;
-      add_str(real_table->alias.str, real_table->alias.length);
-    }
+    /* Derived table name generation */
+    size_t len= my_snprintf(table_name_buffer, sizeof(table_name_buffer)-1,
+                            "<derived%u>",
+                            tab->table->derived_select_number);
+    add_str(table_name_buffer, len);
+  }
+  else if (tab->bush_children)
+  {
+    JOIN_TAB *ctab= tab->bush_children->start;
+    size_t len= my_snprintf(table_name_buffer,
+                            sizeof(table_name_buffer)-1,
+                            "<subquery%d>",
+                            ctab->emb_sj_nest->sj_subq_pred->get_identifier());
+    add_str(table_name_buffer, len);
   }
   else
-    DBUG_ASSERT(0);
+  {
+    TABLE_LIST *real_table= tab->table->pos_in_table_list;
+    add_str(real_table->alias.str, real_table->alias.length);
+  }
 }
 
 void Json_writer::add_table_name(const TABLE *table)

sql/rowid_filter.cc

@@ -39,7 +39,8 @@ double Range_rowid_filter_cost_info::lookup_cost(
 
 /**
   @brief
-    The average gain in cost per row to use the range filter with this cost info
+    The average gain in cost per row to use the range filter with this cost
+    info
 */
 
 inline
@@ -58,8 +59,9 @@ double Range_rowid_filter_cost_info::avg_access_and_eval_gain_per_row(
   @param access_cost_factor the adjusted cost of access a row
 
   @details
-    The current code to estimate the cost of a ref access is quite inconsistent:
-    in some cases the effect of page buffers is taken into account, for others
+    The current code to estimate the cost of a ref access is quite
+    inconsistent:
+    In some cases the effect of page buffers is taken into account, for others
     just the engine dependent read_time() is employed. That's why the average
     cost of one random seek might differ from 1.
     The parameter access_cost_factor can be considered as the cost of a random

sql/sql_join_cache.cc

@@ -1080,7 +1080,7 @@ int JOIN_CACHE::init(bool for_explain)
 
 
 /* 
-  Check the possibility to read the access keys directly from the join buffer       
+  Check the possibility to read the access keys directly from the join buffer
   SYNOPSIS
     check_emb_key_usage()
 

sql/sql_lex.cc

@@ -5956,7 +5956,7 @@ unit_common_op st_select_lex_unit::common_op()
       else
       {
         if (operation != op)
-          operation= OP_MIX;
+          return OP_MIX;
       }
     }
   }
@@ -5966,12 +5966,13 @@ unit_common_op st_select_lex_unit::common_op()
   Save explain structures of a UNION. The only variable member is whether the 
   union has "Using filesort".
 
-  There is also save_union_explain_part2() function, which is called before we read
-  UNION's output.
+  There is also save_union_explain_part2() function, which is called before we
+  read UNION's output.
 
   The reason for it is examples like this:
 
-     SELECT col1 FROM t1 UNION SELECT col2 FROM t2 ORDER BY (select ... from t3 ...)
+     SELECT col1 FROM t1 UNION SELECT col2 FROM t2
+     ORDER BY (select ... from t3 ...)
 
   Here, the (select ... from t3 ...) subquery must be a child of UNION's
   st_select_lex. However, it is not connected as child until a very late 
@@ -10191,7 +10192,7 @@ SELECT_LEX_UNIT *LEX::parsed_select_expr_cont(SELECT_LEX_UNIT *unit,
 
 /**
   Add primary expression as the next term in a given query expression body
-  pruducing a new query expression body
+  producing a new query expression body
 */
 
 SELECT_LEX_UNIT *

sql/sql_select.h

@@ -1343,7 +1343,9 @@ class JOIN :public Sql_alloc
   int dbug_join_tab_array_size;
 #endif
 
-  /* We also maintain a stack of join optimization states in * join->positions[] */
+  /*
+    We also maintain a stack of join optimization states in join->positions[]
+  */
 /******* Join optimization state members end *******/
 
   /*

sql/sql_update.cc

@@ -833,9 +833,9 @@ int mysql_update(THD *thd,
         table->use_all_columns();
 
       /*
-	We are doing a search on a key that is updated. In this case
-	we go trough the matching rows, save a pointer to them and
-	update these in a separate loop based on the pointer.
+        We are doing a search on a key that is updated. In this case
+        we go trough the matching rows, save a pointer to them and
+        update these in a separate loop based on the pointer.
       */
       explain->buf_tracker.on_scan_init();
       IO_CACHE tempfile;