List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:January 26 2012 12:54pm
Subject:bzr push into mysql-trunk branch (ole.john.aske:3798 to 3799) Bug#13330645
View as plain text  
 3799 Ole John Aske	2012-01-26
       Fix for bug#13330645 'MRR_SORTED' IS NOT USED EVEN IF HANDLER NATIVELY SUPPORTS 'HA_MRR_SORTED'
        
       There was an assumption that only the default MRR implementation is 
       able to produce 'HA_MRR_SORTED' results. This has lead to the bad codestyle
       of either setting  'param.force_default_mrr= true' or specifying the
       HA_MRR_USE_DEFAULT_IMPL flag whenever we wanted a sorted MRR resultset.
            
       This fix leaves the decision of using either the native or the default MRR
       implementation to the handler itself based on its capabilities.
            
       Furthermore, mostly due to skip_sort_order not being decided until
       JOIN::exec(), the handler will also set the (new) HA_MRR_SUPPORT_SORTED
       flag to signal if its is 'Using MRR' if its multi_range_read_init()
       is called with the HA_MRR_SORTED flag - This has the sole purpose of
       getting the EXPLAIN correct. This part of the fix might become obsolete
       if/when the planned reshuffle of skip_sort_order (Into JOIN::optimize) 
       has been completed.
            
       There was also a problem with DsMrr_impl::dsmrr_init() not
       checking the OPTIMIZER_SWITCH_MRR when deciding to init()
       either the DS-MRR or default MRR implementation.
       (There are cases where ::dsmrr_init() is called wo/ 
       ::dsmrr_info[_const]() being called first).
            

    modified:
      sql/handler.cc
      sql/handler.h
      sql/opt_explain.cc
      sql/opt_range.cc
 3798 Marko Mäkelä	2012-01-26
      Bug#13610358 innodb_sort_buf_size should be called innodb_sort_buffer_size
      for consistency

    renamed:
      mysql-test/suite/sys_vars/r/innodb_sort_buf_size_basic.result => mysql-test/suite/sys_vars/r/innodb_sort_buffer_size_basic.result
      mysql-test/suite/sys_vars/t/innodb_sort_buf_size_basic.test => mysql-test/suite/sys_vars/t/innodb_sort_buffer_size_basic.test
    modified:
      storage/innobase/handler/ha_innodb.cc
      mysql-test/suite/sys_vars/r/innodb_sort_buffer_size_basic.result
      mysql-test/suite/sys_vars/t/innodb_sort_buffer_size_basic.test
=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2012-01-06 09:03:53 +0000
+++ b/sql/handler.cc	2012-01-26 12:53:38 +0000
@@ -4601,7 +4601,9 @@ handler::multi_range_read_info_const(uin
   if (total_rows != HA_POS_ERROR)
   {
     /* The following calculation is the same as in multi_range_read_info(): */
-    *flags |= HA_MRR_USE_DEFAULT_IMPL;
+    *flags|= HA_MRR_USE_DEFAULT_IMPL;
+    *flags|= HA_MRR_SUPPORT_SORTED;
+
     DBUG_ASSERT(cost->is_zero());
     if ((*flags & HA_MRR_INDEX_ONLY) && total_rows > 2)
       cost->add_io(index_only_read_time(keyno, total_rows) *
@@ -4655,7 +4657,8 @@ ha_rows handler::multi_range_read_info(u
 {
   *bufsz= 0; /* Default implementation doesn't need a buffer */
 
-  *flags |= HA_MRR_USE_DEFAULT_IMPL;
+  *flags|= HA_MRR_USE_DEFAULT_IMPL;
+  *flags|= HA_MRR_SUPPORT_SORTED;
 
   DBUG_ASSERT(cost->is_zero());
 
@@ -4819,13 +4822,15 @@ int DsMrr_impl::dsmrr_init(handler *h_ar
   handler *new_h2= 0;
   int retval= 0;
   DBUG_ENTER("DsMrr_impl::dsmrr_init");
+  THD *thd= current_thd;
 
   /*
     index_merge may invoke a scan on an object for which dsmrr_info[_const]
     has not been called, so set the owner handler here as well.
   */
   h= h_arg;
-  if (mode & HA_MRR_USE_DEFAULT_IMPL || mode & HA_MRR_SORTED)
+  if (!thd->optimizer_switch_flag(OPTIMIZER_SWITCH_MRR) ||
+      mode & (HA_MRR_USE_DEFAULT_IMPL | HA_MRR_SORTED)) // DS-MRR doesn't sort
   {
     use_default_impl= TRUE;
     retval= h->handler::multi_range_read_init(seq_funcs, seq_init_param,
@@ -4875,7 +4880,6 @@ int DsMrr_impl::dsmrr_init(handler *h_ar
   if (!h2)
   {
     /* Create a separate handler object to do rndpos() calls. */
-    THD *thd= current_thd;
     /*
       ::clone() takes up a lot of stack, especially on 64 bit platforms.
       The constant 5 is an empiric result.
@@ -5155,6 +5159,7 @@ ha_rows DsMrr_impl::dsmrr_info(uint keyn
     DBUG_PRINT("info", ("Default MRR implementation choosen"));
     *flags= def_flags;
     *bufsz= def_bufsz;
+    DBUG_ASSERT(*flags & HA_MRR_USE_DEFAULT_IMPL);
   }
   else
   {
@@ -5198,6 +5203,7 @@ ha_rows DsMrr_impl::dsmrr_info_const(uin
     DBUG_PRINT("info", ("Default MRR implementation choosen"));
     *flags= def_flags;
     *bufsz= def_bufsz;
+    DBUG_ASSERT(*flags & HA_MRR_USE_DEFAULT_IMPL);
   }
   else
   {
@@ -5237,12 +5243,11 @@ bool DsMrr_impl::choose_mrr_impl(uint ke
   bool res;
   THD *thd= current_thd;
   if (!thd->optimizer_switch_flag(OPTIMIZER_SWITCH_MRR) ||
-      *flags & HA_MRR_INDEX_ONLY ||
+      *flags & (HA_MRR_INDEX_ONLY | HA_MRR_SORTED) || // Unsupported by DS-MRR
       (keyno == table->s->primary_key && h->primary_key_is_clustered()) ||
        key_uses_partial_cols(table, keyno))
   {
-    /* Use the default implementation */
-    *flags |= HA_MRR_USE_DEFAULT_IMPL;
+    /* Use the default implementation, don't modify args: See comments  */
     return TRUE;
   }
   
@@ -5266,7 +5271,7 @@ bool DsMrr_impl::choose_mrr_impl(uint ke
   if (force_dsmrr || (dsmrr_cost.total_cost() <= cost->total_cost()))
   {
     *flags &= ~HA_MRR_USE_DEFAULT_IMPL;  /* Use the DS-MRR implementation */
-    *flags &= ~HA_MRR_SORTED;          /* We will return unordered output */
+    *flags &= ~HA_MRR_SUPPORT_SORTED;    /* We can't provide ordered output */
     *cost= dsmrr_cost;
     res= FALSE;
   }

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2011-11-30 11:36:14 +0000
+++ b/sql/handler.h	2012-01-26 12:53:38 +0000
@@ -1163,6 +1163,10 @@ void get_sweep_read_cost(TABLE *table, h
 /* 
   The MRR user will provide ranges in key order, and MRR implementation
   must return rows in key order.
+  Passing this flag to multi_read_range_init() may cause the
+  default MRR handler to be used even if HA_MRR_USE_DEFAULT_IMPL
+  was not specified.
+  (If the native MRR impl. can not provide SORTED result)
 */
 #define HA_MRR_SORTED 8
 
@@ -1191,6 +1195,15 @@ void get_sweep_read_cost(TABLE *table, h
 */
 #define HA_MRR_NO_NULL_ENDPOINTS 128
 
+/*
+  Set by the MRR implementation to signal that it will natively
+  produced sorted result if multi_range_read_init() is called with
+  the HA_MRR_SORTED flag - Else multi_range_read_init(HA_MRR_SORTED)
+  will revert to use the default MRR implementation. 
+*/
+#define HA_MRR_SUPPORT_SORTED 256
+
+
 class ha_statistics
 {
 public:

=== modified file 'sql/opt_explain.cc'
--- a/sql/opt_explain.cc	2011-12-14 12:32:55 +0000
+++ b/sql/opt_explain.cc	2012-01-26 12:53:38 +0000
@@ -770,24 +770,25 @@ void Explain_table_base::explain_extra_c
   if (table->reginfo.not_exists_optimize)
     str_extra->append(STRING_WITH_LEN("; Not exists"));
 
-  if (quick_type == QUICK_SELECT_I::QS_TYPE_RANGE &&
-      !(((QUICK_RANGE_SELECT*)(select->quick))->mrr_flags &
-       (HA_MRR_USE_DEFAULT_IMPL | HA_MRR_SORTED)))
+  if (quick_type == QUICK_SELECT_I::QS_TYPE_RANGE)
   {
+    uint mrr_flags=
+      ((QUICK_RANGE_SELECT*)(select->quick))->mrr_flags;
+
     /*
       During normal execution of a query, multi_range_read_init() is
       called to initialize MRR. If HA_MRR_SORTED is set at this point,
       multi_range_read_init() for any native MRR implementation will
-      revert to default MRR because they cannot produce sorted output
-      currently.
+      revert to default MRR if not HA_MRR_SUPPORT_SORTED.
       Calling multi_range_read_init() can potentially be costly, so it
-      is not done when executing an EXPLAIN. We therefore make the
-      assumption that HA_MRR_SORTED means no MRR. If some MRR native
-      implementation will support sorted output in the future, a
-      function "bool mrr_supports_sorted()" should be added in the
-      handler.
+      is not done when executing an EXPLAIN. We therefore simulate
+      its effect here:
     */
-    str_extra->append(STRING_WITH_LEN("; Using MRR"));
+    if (mrr_flags & HA_MRR_SORTED && !(mrr_flags & HA_MRR_SUPPORT_SORTED))
+      mrr_flags|= HA_MRR_USE_DEFAULT_IMPL;
+
+    if (!(mrr_flags & HA_MRR_USE_DEFAULT_IMPL))
+      str_extra->append(STRING_WITH_LEN("; Using MRR"));
   }
 }
 

=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2012-01-26 10:08:12 +0000
+++ b/sql/opt_range.cc	2012-01-26 12:53:38 +0000
@@ -2568,7 +2568,7 @@ int SQL_SELECT::test_quick_select(THD *t
     param.imerge_cost_buff_size= 0;
     param.using_real_indexes= TRUE;
     param.remove_jump_scans= TRUE;
-    param.force_default_mrr= (interesting_order != ORDER::ORDER_NOT_RELEVANT);
+    param.force_default_mrr= (interesting_order == ORDER::ORDER_DESC);
     param.order_direction= interesting_order;
 
     thd->no_errors=1;				// Don't warn about NULL
@@ -5562,7 +5562,7 @@ QUICK_SELECT_I *TRP_ROR_INTERSECT::make_
     {
       if (!(quick= get_quick_select(param, (*current)->idx,
                                     (*current)->sel_arg,
-                                    HA_MRR_USE_DEFAULT_IMPL | HA_MRR_SORTED,
+                                    HA_MRR_SORTED,
                                     0, alloc)) ||
           quick_intrsect->push_quick_back(quick))
       {
@@ -5574,7 +5574,7 @@ QUICK_SELECT_I *TRP_ROR_INTERSECT::make_
     {
       if (!(quick= get_quick_select(param, cpk_scan->idx,
                                     cpk_scan->sel_arg,
-                                    HA_MRR_USE_DEFAULT_IMPL | HA_MRR_SORTED,
+                                    HA_MRR_SORTED,
                                     0, alloc)))
       {
         delete quick_intrsect;
@@ -8705,10 +8705,12 @@ ha_rows check_quick_select(PARAM *param,
     param->is_ror_scan= FALSE;
   
   *mrr_flags= param->force_default_mrr? HA_MRR_USE_DEFAULT_IMPL: 0;
+  *mrr_flags|= HA_MRR_NO_ASSOCIATION;
   /*
     Pass HA_MRR_SORTED to see if MRR implementation can handle sorting.
   */
-  *mrr_flags|= HA_MRR_NO_ASSOCIATION | HA_MRR_SORTED;
+  if (param->order_direction != ORDER::ORDER_NOT_RELEVANT)
+    *mrr_flags|= HA_MRR_SORTED;
 
   bool pk_is_clustered= file->primary_key_is_clustered();
   if (index_only && 
@@ -8717,7 +8719,7 @@ ha_rows check_quick_select(PARAM *param,
      *mrr_flags |= HA_MRR_INDEX_ONLY;
   
   if (current_thd->lex->sql_command != SQLCOM_SELECT)
-    *mrr_flags |= HA_MRR_USE_DEFAULT_IMPL;
+    *mrr_flags|= HA_MRR_SORTED; // Assumed to give faster ins/upd/del
 
   *bufsize= param->thd->variables.read_rnd_buff_size;
   // Sets is_ror_scan to false for some queries, e.g. multi-ranges
@@ -9242,7 +9244,7 @@ QUICK_RANGE_SELECT *get_quick_select_for
   quick->mrr_flags= HA_MRR_NO_ASSOCIATION | 
                     (table->key_read ? HA_MRR_INDEX_ONLY : 0);
   if (thd->lex->sql_command != SQLCOM_SELECT)
-    quick->mrr_flags |= HA_MRR_USE_DEFAULT_IMPL;
+    quick->mrr_flags|= HA_MRR_SORTED; // Assumed to give faster ins/upd/del
 #ifdef WITH_NDBCLUSTER_STORAGE_ENGINE
   if (!ref->null_ref_key && !key_has_nulls(key_info, range->min_key,
                                            ref->key_length))
@@ -10929,7 +10931,7 @@ get_best_group_min_max(PARAM *param, SEL
                                            &cur_param_idx);
       /* Check if this range tree can be used for prefix retrieval. */
       Cost_estimate dummy_cost;
-      uint mrr_flags= HA_MRR_USE_DEFAULT_IMPL;
+      uint mrr_flags= HA_MRR_SORTED;
       uint mrr_bufsize=0;
       cur_quick_prefix_records= check_quick_select(param, cur_param_idx, 
                                                    FALSE /*don't care*/, 
@@ -11564,7 +11566,6 @@ TRP_GROUP_MIN_MAX::make_quick(PARAM *par
       /* Make a QUICK_RANGE_SELECT to be used for group prefix retrieval. */
       quick->quick_prefix_select= get_quick_select(param, param_idx,
                                                    index_tree,
-                                                   HA_MRR_USE_DEFAULT_IMPL |
                                                    HA_MRR_SORTED,
                                                    0,
                                                    &quick->alloc);

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (ole.john.aske:3798 to 3799) Bug#13330645Ole John Aske30 Jan