bzr commit into mysql-5.1 branch (ole.john.aske:3473) Bug#57601

[Ole John Aske]

#At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.1/ based on revid:sunanda.menon@stripped

 3473 Ole John Aske	2010-10-20
      Proposed fix for bug#57601 'Optimizer is overly eager to request ordered access.'
      
      1)  Ensure that JOIN_TAB::sorted and QUICK_SELECT_I::sorted is requested only when
          strict ordering is required from the handler - Either as a result of the query
          specifying ORDER/GROUP BY, or the handler being a source in a QUICK access
          method which require the sources to be ordered
      
      2a Call handler::ha_index_init(int idx, bool sorted) with 'sorted==false' in any
         access methods requesting a single row (HA_READ_KEY_EXACT)
         (join_read_key(), .....
      
      2b) Else: Always use above 'sorted' attributes as arguments to handler::ha_index_init()
          and other handler methods having a 'bool sorted' argument (::read_range_first())

    modified:
      sql/opt_range.cc
      sql/opt_range.h
      sql/sql_select.cc
=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2010-08-24 15:51:32 +0000
+++ b/sql/opt_range.cc	2010-10-20 13:22:47 +0000
@@ -4980,7 +4980,7 @@ QUICK_SELECT_I *TRP_ROR_INTERSECT::make_
     {
       if (!(quick= get_quick_select(param, (*first_scan)->idx,
                                     (*first_scan)->sel_arg, alloc)) ||
-          quick_intrsect->push_quick_back(quick))
+          (quick->sorted= 1, quick_intrsect->push_quick_back(quick)))
       {
         delete quick_intrsect;
         DBUG_RETURN(NULL);
@@ -4995,6 +4995,7 @@ QUICK_SELECT_I *TRP_ROR_INTERSECT::make_
         DBUG_RETURN(NULL);
       }
       quick->file= NULL; 
+      quick->sorted= 1;
       quick_intrsect->cpk_quick= quick;
     }
     quick_intrsect->records= records;
@@ -5021,7 +5022,7 @@ QUICK_SELECT_I *TRP_ROR_UNION::make_quic
     for (scan= first_ror; scan != last_ror; scan++)
     {
       if (!(quick= (*scan)->make_quick(param, FALSE, &quick_roru->alloc)) ||
-          quick_roru->push_quick_back(quick))
+          (quick->sorted= 1, quick_roru->push_quick_back(quick)))
         DBUG_RETURN(NULL);
     }
     quick_roru->records= records;
@@ -8451,7 +8452,7 @@ int QUICK_RANGE_SELECT::reset()
   in_range= FALSE;
   cur_range= (QUICK_RANGE**) ranges.buffer;
 
-  if (file->inited == handler::NONE && (error= file->ha_index_init(index,1)))
+  if (file->inited == handler::NONE && (error= file->ha_index_init(index,sorted)))
     DBUG_RETURN(error);
  
   /* Do not allocate the buffers twice. */
@@ -8668,7 +8669,7 @@ int QUICK_RANGE_SELECT::get_next_prefix(
     result= file->read_range_first(last_range->min_keypart_map ? &start_key : 0,
 				   last_range->max_keypart_map ? &end_key : 0,
                                    test(last_range->flag & EQ_RANGE),
-				   TRUE);
+				   sorted);
     if (last_range->flag == (UNIQUE_RANGE | EQ_RANGE))
       last_range= 0;			// Stop searching
 
@@ -8794,6 +8795,7 @@ QUICK_SELECT_DESC::QUICK_SELECT_DESC(QUI
       r->flag&= ~EQ_RANGE;
   }
   rev_it.rewind();
+  sorted= 1;
   q->dont_free=1;				// Don't free shared mem
   delete q;
 }
@@ -10205,11 +10207,18 @@ TRP_GROUP_MIN_MAX::make_quick(PARAM *par
     if (quick_prefix_records == HA_POS_ERROR)
       quick->quick_prefix_select= NULL; /* Can't construct a quick select. */
     else
+    {
       /* Make a QUICK_RANGE_SELECT to be used for group prefix retrieval. */
       quick->quick_prefix_select= get_quick_select(param, param_idx,
                                                    index_tree,
                                                    &quick->alloc);
-
+      if (!quick->quick_prefix_select)
+      {
+        delete quick;
+        DBUG_RETURN(NULL);
+      }
+      quick->quick_prefix_select->sorted= 1;
+    }
     /*
       Extract the SEL_ARG subtree that contains only ranges for the MIN/MAX
       attribute, and create an array of QUICK_RANGES to be used by the
@@ -10600,6 +10609,9 @@ int QUICK_GROUP_MIN_MAX_SELECT::reset(vo
   DBUG_ENTER("QUICK_GROUP_MIN_MAX_SELECT::reset");
 
   head->set_keyread(TRUE); /* We need only the key attributes */
+
+  // Request ordered index access as usage of ::index_last(), ::index_first()
+  // within GROUP_MIN_MAX depends on it.
   if ((result= file->ha_index_init(index,1)))
     DBUG_RETURN(result);
   if (quick_prefix_select && quick_prefix_select->reset())

=== modified file 'sql/opt_range.h'
--- a/sql/opt_range.h	2010-07-19 09:03:52 +0000
+++ b/sql/opt_range.h	2010-10-20 13:22:47 +0000
@@ -347,7 +347,7 @@ class SEL_ARG;
 
 /*
   Quick select that does a range scan on a single key. The records are
-  returned in key order.
+  returned in key order if 'QUICK_SELECT_I::sorted==true'.
 */
 class QUICK_RANGE_SELECT : public QUICK_SELECT_I
 {

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2010-09-02 00:43:02 +0000
+++ b/sql/sql_select.cc	2010-10-20 13:22:47 +0000
@@ -1742,6 +1742,17 @@ JOIN::save_join_tab()
 }
 
 
+static void
+disable_sorted_access(JOIN_TAB* join_tab)
+{
+  join_tab->sorted= 0;
+  if (join_tab->select && join_tab->select->quick)
+  {
+    join_tab->select->quick->sorted= 0;
+  }
+}
+
+
 /**
   Exec select.
 
@@ -1911,7 +1922,7 @@ JOIN::exec()
     DBUG_PRINT("info", ("%s", thd->proc_info));
     if (!curr_join->sort_and_group &&
         curr_join->const_tables != curr_join->tables)
-      curr_join->join_tab[curr_join->const_tables].sorted= 0;
+      disable_sorted_access(&curr_join->join_tab[curr_join->const_tables]);
     if ((tmp_error= do_select(curr_join, (List<Item> *) 0, curr_tmp_table, 0)))
     {
       error= tmp_error;
@@ -2075,7 +2086,7 @@ JOIN::exec()
       curr_join->group_list= 0;
       if (!curr_join->sort_and_group &&
           curr_join->const_tables != curr_join->tables)
-        curr_join->join_tab[curr_join->const_tables].sorted= 0;
+        disable_sorted_access(&curr_join->join_tab[curr_join->const_tables]);
       if (setup_sum_funcs(curr_join->thd, curr_join->sum_funcs) ||
 	  (tmp_error= do_select(curr_join, (List<Item> *) 0, curr_tmp_table,
 				0)))
@@ -6658,7 +6669,7 @@ make_join_readinfo(JOIN *join, ulonglong
   uint i;
   bool statistics= test(!(join->select_options & SELECT_DESCRIBE));
   bool ordered_set= 0;
-  bool sorted= 1;
+  bool sorted= (join->order || join->group_list);
   DBUG_ENTER("make_join_readinfo");
 
   for (i=join->const_tables ; i < join->tables ; i++)
@@ -11911,7 +11922,7 @@ join_read_key(JOIN_TAB *tab)
 
   if (!table->file->inited)
   {
-    table->file->ha_index_init(tab->ref.key, tab->sorted);
+    table->file->ha_index_init(tab->ref.key, 0);
   }
   if (cmp_buffer_with_ref(tab) ||
       (table->status & (STATUS_GARBAGE | STATUS_NO_PARENT | STATUS_NULL_ROW)))
@@ -12183,7 +12194,7 @@ join_read_last(JOIN_TAB *tab)
   tab->read_record.index=tab->index;
   tab->read_record.record=table->record[0];
   if (!table->file->inited)
-    table->file->ha_index_init(tab->index, 1);
+    table->file->ha_index_init(tab->index, tab->sorted);
   if ((error= tab->table->file->index_last(tab->table->record[0])))
     return report_error(table, error);
   return 0;
@@ -12207,7 +12218,7 @@ join_ft_read_first(JOIN_TAB *tab)
   TABLE *table= tab->table;
 
   if (!table->file->inited)
-    table->file->ha_index_init(tab->ref.key, 1);
+    table->file->ha_index_init(tab->ref.key, tab->sorted);
 #if NOT_USED_YET
   /* as ft-key doesn't use store_key's, see also FT_SELECT::init() */
   if (cp_buffer_from_ref(tab->join->thd, table, &tab->ref))


Attachment: [text/bzr-bundle] bzr/[e-mail]