List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:January 12 2011 8:01am
Subject:bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3244) WL#5257
View as plain text  
#At file:///export/home/didrik/repo/next-mr-opt-backporting-wl4800/ based on revid:tor.didriksen@stripped

 3244 Tor Didriksen	2011-01-12
      WL#5257 Review comments.
      
      remove a bunch of goto end-of-function, just return instead.
      this also allows us to remove the extra scope introduced,
      making the patch about 300 lines smaller compared to the original in trunk.

    modified:
      sql/opt_range.cc
=== modified file 'sql/opt_range.cc'
--- a/sql/opt_range.cc	2011-01-11 08:15:46 +0000
+++ b/sql/opt_range.cc	2011-01-12 08:00:09 +0000
@@ -10099,23 +10099,23 @@ get_best_group_min_max(PARAM *param, SEL
   /* Perform few 'cheap' tests whether this access method is applicable. */
   if (!join)
   {
-    cause= "no_join";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "no_join");
+    DBUG_RETURN(NULL);
   }
   if (join->tables != 1)   /* The query must reference one table. */
   {
-    cause= "not_single_table";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "not_single_table");
+    DBUG_RETURN(NULL);
   }
   if (join->select_lex->olap == ROLLUP_TYPE) /* Check (B3) for ROLLUP */
   {
-    cause= "rollup";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "rollup");
+    DBUG_RETURN(NULL);
   }
   if (table->s->keys == 0)        /* There are no indexes to use. */
   {
-    cause= "no_index";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", "no_index");
+    DBUG_RETURN(NULL);
   }
 
   /* Check (SA1,SA4) and store the only MIN/MAX argument - the C attribute.*/
@@ -10129,7 +10129,8 @@ get_best_group_min_max(PARAM *param, SEL
       !is_agg_distinct)
   {
     cause= "not_group_by_or_distinct";
-    goto return_null_not_chosen_cause;
+    trace_group.add("chosen", false).add_alnum("cause", cause);
+    DBUG_RETURN(NULL);
   }
   /* Analyze the query in more detail. */
 
@@ -10150,7 +10151,8 @@ get_best_group_min_max(PARAM *param, SEL
       else
       {
         cause= "not_applicable_aggregate_function";
-        goto return_null_not_chosen_cause;
+        trace_group.add("chosen", false).add_alnum("cause", cause);
+        DBUG_RETURN(NULL);
       }
 
       /* The argument of MIN/MAX. */
@@ -10185,7 +10187,8 @@ get_best_group_min_max(PARAM *param, SEL
     if ((*tmp_group->item)->real_item()->type() != Item::FIELD_ITEM)
     {
       cause= "group_field_is_expression";
-      goto return_null_not_chosen_cause;
+      trace_group.add("chosen", false).add_alnum("cause", cause);
+      DBUG_RETURN(NULL);
     }
   }
 
@@ -10194,337 +10197,339 @@ get_best_group_min_max(PARAM *param, SEL
     (GA1,GA2) are all TRUE. If there is more than one such index, select the
     first one. Here we set the variables: group_prefix_len and index_info.
   */
-  {
-    const uint pk= param->table->s->primary_key;
-    KEY *cur_index_info= table->key_info;
-    KEY *cur_index_info_end= cur_index_info + table->s->keys;
-    SEL_ARG *cur_index_tree= NULL;
-    ha_rows cur_quick_prefix_records= 0;
-    uint cur_param_idx= MAX_KEY;
-    Opt_trace_array ota(thd->opt_trace, "potential_group_range_indices");
-    for (uint cur_index= 0 ; cur_index_info != cur_index_info_end ;
-         cur_index_info++, cur_index++)
-    {
-      Opt_trace_object trace_idx(thd->opt_trace);
-      trace_idx.add_utf8("index", cur_index_info->name);
-      KEY_PART_INFO *cur_part;
-      KEY_PART_INFO *end_part; /* Last part for loops. */
-      /* Last index part. */
-      KEY_PART_INFO *last_part;
-      KEY_PART_INFO *first_non_group_part;
-      KEY_PART_INFO *first_non_infix_part;
-      uint key_infix_parts;
-      uint cur_group_key_parts= 0;
-      uint cur_group_prefix_len= 0;
-      double cur_read_cost;
-      ha_rows cur_records;
-      key_map used_key_parts_map;
-      uint max_key_part= 0;
-      uint cur_key_infix_len= 0;
-      uchar cur_key_infix[MAX_KEY_LENGTH];
-      uint cur_used_key_parts;
+
+  const uint pk= param->table->s->primary_key;
+  KEY *cur_index_info= table->key_info;
+  KEY *cur_index_info_end= cur_index_info + table->s->keys;
+  SEL_ARG *cur_index_tree= NULL;
+  ha_rows cur_quick_prefix_records= 0;
+  uint cur_param_idx= MAX_KEY;
+  Opt_trace_array ota(thd->opt_trace, "potential_group_range_indices");
+  for (uint cur_index= 0 ; cur_index_info != cur_index_info_end ;
+       cur_index_info++, cur_index++)
+  {
+    Opt_trace_object trace_idx(thd->opt_trace);
+    trace_idx.add_utf8("index", cur_index_info->name);
+    KEY_PART_INFO *cur_part;
+    KEY_PART_INFO *end_part; /* Last part for loops. */
+    /* Last index part. */
+    KEY_PART_INFO *last_part;
+    KEY_PART_INFO *first_non_group_part;
+    KEY_PART_INFO *first_non_infix_part;
+    uint key_infix_parts;
+    uint cur_group_key_parts= 0;
+    uint cur_group_prefix_len= 0;
+    double cur_read_cost;
+    ha_rows cur_records;
+    key_map used_key_parts_map;
+    uint max_key_part= 0;
+    uint cur_key_infix_len= 0;
+    uchar cur_key_infix[MAX_KEY_LENGTH];
+    uint cur_used_key_parts;
     
-      /* Check (B1) - if current index is covering. */
-      if (!table->covering_keys.is_set(cur_index))
-      {
-        cause= "not_covering";
-        goto next_index;
-      }
+    /* Check (B1) - if current index is covering. */
+    if (!table->covering_keys.is_set(cur_index))
+    {
+      cause= "not_covering";
+      goto next_index;
+    }
 
-      /*
-        If the current storage manager is such that it appends the primary key to
-        each index, then the above condition is insufficient to check if the
-        index is covering. In such cases it may happen that some fields are
-        covered by the PK index, but not by the current index. Since we can't
-        use the concatenation of both indexes for index lookup, such an index
-        does not qualify as covering in our case. If this is the case, below
-        we check that all query fields are indeed covered by 'cur_index'.
-      */
-      if (pk < MAX_KEY && cur_index != pk &&
-          (table->file->ha_table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX))
+    /*
+      If the current storage manager is such that it appends the primary key to
+      each index, then the above condition is insufficient to check if the
+      index is covering. In such cases it may happen that some fields are
+      covered by the PK index, but not by the current index. Since we can't
+      use the concatenation of both indexes for index lookup, such an index
+      does not qualify as covering in our case. If this is the case, below
+      we check that all query fields are indeed covered by 'cur_index'.
+    */
+    if (pk < MAX_KEY && cur_index != pk &&
+        (table->file->ha_table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX))
+    {
+      /* For each table field */
+      for (uint i= 0; i < table->s->fields; i++)
       {
-        /* For each table field */
-        for (uint i= 0; i < table->s->fields; i++)
+        Field *cur_field= table->field[i];
+        /*
+          If the field is used in the current query ensure that it's
+          part of 'cur_index'
+        */
+        if (bitmap_is_set(table->read_set, cur_field->field_index) &&
+            !cur_field->part_of_key_not_clustered.is_set(cur_index))
         {
-          Field *cur_field= table->field[i];
-          /*
-            If the field is used in the current query ensure that it's
-            part of 'cur_index'
-          */
-          if (bitmap_is_set(table->read_set, cur_field->field_index) &&
-              !cur_field->part_of_key_not_clustered.is_set(cur_index))
-          {
-            cause= "not_covering";
-            goto next_index;                  // Field was not part of key
-          }
+          cause= "not_covering";
+          goto next_index;                  // Field was not part of key
         }
       }
-      trace_idx.add("covering", true);
+    }
+    trace_idx.add("covering", true);
 
-      /*
-        Check (GA1) for GROUP BY queries.
-      */
-      if (join->group_list)
+    /*
+      Check (GA1) for GROUP BY queries.
+    */
+    if (join->group_list)
+    {
+      cur_part= cur_index_info->key_part;
+      end_part= cur_part + cur_index_info->key_parts;
+      /* Iterate in parallel over the GROUP list and the index parts. */
+      for (tmp_group= join->group_list; tmp_group && (cur_part != end_part);
+           tmp_group= tmp_group->next, cur_part++)
       {
-        cur_part= cur_index_info->key_part;
-        end_part= cur_part + cur_index_info->key_parts;
-        /* Iterate in parallel over the GROUP list and the index parts. */
-        for (tmp_group= join->group_list; tmp_group && (cur_part != end_part);
-             tmp_group= tmp_group->next, cur_part++)
+        /*
+          TODO:
+          tmp_group::item is an array of Item, is it OK to consider only the
+          first Item? If so, then why? What is the array for?
+        */
+        /* Above we already checked that all group items are fields. */
+        DBUG_ASSERT((*tmp_group->item)->real_item()->type() == Item::FIELD_ITEM);
+        Item_field *group_field= (Item_field *) (*tmp_group->item)->real_item();
+        if (group_field->field->eq(cur_part->field))
         {
-          /*
-            TODO:
-            tmp_group::item is an array of Item, is it OK to consider only the
-            first Item? If so, then why? What is the array for?
-          */
-          /* Above we already checked that all group items are fields. */
-          DBUG_ASSERT((*tmp_group->item)->real_item()->type() == Item::FIELD_ITEM);
-          Item_field *group_field= (Item_field *) (*tmp_group->item)->real_item();
-          if (group_field->field->eq(cur_part->field))
-          {
-            cur_group_prefix_len+= cur_part->store_length;
-            ++cur_group_key_parts;
-            max_key_part= cur_part - cur_index_info->key_part + 1;
-            used_key_parts_map.set_bit(max_key_part);
-          }
-          else
-          {
-            cause= "group_attribute_not_prefix_in_index";
-            goto next_index;
-          }
+          cur_group_prefix_len+= cur_part->store_length;
+          ++cur_group_key_parts;
+          max_key_part= cur_part - cur_index_info->key_part + 1;
+          used_key_parts_map.set_bit(max_key_part);
+        }
+        else
+        {
+          cause= "group_attribute_not_prefix_in_index";
+          goto next_index;
         }
       }
+    }
 
-      /*
-        Check (GA2) if this is a DISTINCT query.
-        If GA2, then Store a new ORDER object in group_fields_array at the
-        position of the key part of item_field->field. Thus we get the ORDER
-        objects for each field ordered as the corresponding key parts.
-        Later group_fields_array of ORDER objects is used to convert the query
-        to a GROUP query.
-      */
-      if ((!join->group_list && join->select_distinct) ||
-          is_agg_distinct)
+    /*
+      Check (GA2) if this is a DISTINCT query.
+      If GA2, then Store a new ORDER object in group_fields_array at the
+      position of the key part of item_field->field. Thus we get the ORDER
+      objects for each field ordered as the corresponding key parts.
+      Later group_fields_array of ORDER objects is used to convert the query
+      to a GROUP query.
+    */
+    if ((!join->group_list && join->select_distinct) ||
+        is_agg_distinct)
+    {
+      if (!is_agg_distinct)
       {
-        if (!is_agg_distinct)
-        {
-          select_items_it.rewind();
-        }
+        select_items_it.rewind();
+      }
+
+      List_iterator<Item_field> agg_distinct_flds_it (agg_distinct_flds);
+      while (NULL != (item = (is_agg_distinct ?
+                              (Item *) agg_distinct_flds_it++ : select_items_it++)))
+      {
+        /* (SA5) already checked above. */
+        item_field= (Item_field*) item->real_item(); 
+        DBUG_ASSERT(item->real_item()->type() == Item::FIELD_ITEM);
 
-        List_iterator<Item_field> agg_distinct_flds_it (agg_distinct_flds);
-        while (NULL != (item = (is_agg_distinct ?
-                                (Item *) agg_distinct_flds_it++ : select_items_it++)))
+        /* not doing loose index scan for derived tables */
+        if (!item_field->field)
         {
-          /* (SA5) already checked above. */
-          item_field= (Item_field*) item->real_item(); 
-          DBUG_ASSERT(item->real_item()->type() == Item::FIELD_ITEM);
-
-          /* not doing loose index scan for derived tables */
-          if (!item_field->field)
-          {
-            cause= "derived_table";
-            goto next_index;
-          }
-
-          /* Find the order of the key part in the index. */
-          key_part_nr= get_field_keypart(cur_index_info, item_field->field);
-          /*
-            Check if this attribute was already present in the select list.
-            If it was present, then its corresponding key part was alredy used.
-          */
-          if (used_key_parts_map.is_set(key_part_nr))
-            continue;
-          if (key_part_nr < 1 ||
-              (!is_agg_distinct && key_part_nr > join->fields_list.elements))
-          {
-            cause= "select_attribute_not_prefix_in_index";
-            goto next_index;
-          }
-          cur_part= cur_index_info->key_part + key_part_nr - 1;
-          cur_group_prefix_len+= cur_part->store_length;
-          used_key_parts_map.set_bit(key_part_nr);
-          ++cur_group_key_parts;
-          max_key_part= max(max_key_part,key_part_nr);
+          cause= "derived_table";
+          goto next_index;
         }
+
+        /* Find the order of the key part in the index. */
+        key_part_nr= get_field_keypart(cur_index_info, item_field->field);
         /*
-          Check that used key parts forms a prefix of the index.
-          To check this we compare bits in all_parts and cur_parts.
-          all_parts have all bits set from 0 to (max_key_part-1).
-          cur_parts have bits set for only used keyparts.
+          Check if this attribute was already present in the select list.
+          If it was present, then its corresponding key part was alredy used.
         */
-        ulonglong all_parts, cur_parts;
-        all_parts= (1<<max_key_part) - 1;
-        cur_parts= used_key_parts_map.to_ulonglong() >> 1;
-        if (all_parts != cur_parts)
-          goto next_index;
-      }
-
-      /* Check (SA2). */
-      if (min_max_arg_item)
-      {
-        key_part_nr= get_field_keypart(cur_index_info, min_max_arg_item->field);
-        if (key_part_nr <= cur_group_key_parts)
+        if (used_key_parts_map.is_set(key_part_nr))
+          continue;
+        if (key_part_nr < 1 ||
+            (!is_agg_distinct && key_part_nr > join->fields_list.elements))
         {
-          cause= "aggregate_column_not_suffix_in_idx";
+          cause= "select_attribute_not_prefix_in_index";
           goto next_index;
         }
-        min_max_arg_part= cur_index_info->key_part + key_part_nr - 1;
+        cur_part= cur_index_info->key_part + key_part_nr - 1;
+        cur_group_prefix_len+= cur_part->store_length;
+        used_key_parts_map.set_bit(key_part_nr);
+        ++cur_group_key_parts;
+        max_key_part= max(max_key_part,key_part_nr);
       }
-
       /*
-        Check (NGA1, NGA2) and extract a sequence of constants to be used as part
-        of all search keys.
+        Check that used key parts forms a prefix of the index.
+        To check this we compare bits in all_parts and cur_parts.
+        all_parts have all bits set from 0 to (max_key_part-1).
+        cur_parts have bits set for only used keyparts.
       */
+      ulonglong all_parts, cur_parts;
+      all_parts= (1<<max_key_part) - 1;
+      cur_parts= used_key_parts_map.to_ulonglong() >> 1;
+      if (all_parts != cur_parts)
+        goto next_index;
+    }
 
-      /*
-        If there is MIN/MAX, each keypart between the last group part and the
-        MIN/MAX part must participate in one equality with constants, and all
-        keyparts after the MIN/MAX part must not be referenced in the query.
-
-        If there is no MIN/MAX, the keyparts after the last group part can be
-        referenced only in equalities with constants, and the referenced keyparts
-        must form a sequence without any gaps that starts immediately after the
-        last group keypart.
-      */
-      last_part= cur_index_info->key_part + cur_index_info->key_parts;
-      first_non_group_part= (cur_group_key_parts < cur_index_info->key_parts) ?
-        cur_index_info->key_part + cur_group_key_parts :
-        NULL;
-      first_non_infix_part= min_max_arg_part ?
-        (min_max_arg_part < last_part) ?
-        min_max_arg_part :
-        NULL :
-        NULL;
-      if (first_non_group_part &&
-          (!min_max_arg_part || (min_max_arg_part - first_non_group_part > 0)))
+    /* Check (SA2). */
+    if (min_max_arg_item)
+    {
+      key_part_nr= get_field_keypart(cur_index_info, min_max_arg_item->field);
+      if (key_part_nr <= cur_group_key_parts)
       {
-        if (tree)
-        {
-          uint dummy;
-          SEL_ARG *index_range_tree= get_index_range_tree(cur_index, tree, param,
-                                                          &dummy);
-          if (!get_constant_key_infix(cur_index_info, index_range_tree,
-                                      first_non_group_part, min_max_arg_part,
-                                      last_part, thd, cur_key_infix, 
-                                      &cur_key_infix_len,
-                                      &first_non_infix_part))
-          {
-            cause= "nonconst_equality_gap_attribute";
-            goto next_index;
-          }
-        }
-        else if (min_max_arg_part &&
-                 (min_max_arg_part - first_non_group_part > 0))
+        cause= "aggregate_column_not_suffix_in_idx";
+        goto next_index;
+      }
+      min_max_arg_part= cur_index_info->key_part + key_part_nr - 1;
+    }
+
+    /*
+      Check (NGA1, NGA2) and extract a sequence of constants to be used as part
+      of all search keys.
+    */
+
+    /*
+      If there is MIN/MAX, each keypart between the last group part and the
+      MIN/MAX part must participate in one equality with constants, and all
+      keyparts after the MIN/MAX part must not be referenced in the query.
+
+      If there is no MIN/MAX, the keyparts after the last group part can be
+      referenced only in equalities with constants, and the referenced keyparts
+      must form a sequence without any gaps that starts immediately after the
+      last group keypart.
+    */
+    last_part= cur_index_info->key_part + cur_index_info->key_parts;
+    first_non_group_part= (cur_group_key_parts < cur_index_info->key_parts) ?
+      cur_index_info->key_part + cur_group_key_parts :
+      NULL;
+    first_non_infix_part= min_max_arg_part ?
+      (min_max_arg_part < last_part) ?
+      min_max_arg_part :
+      NULL :
+      NULL;
+    if (first_non_group_part &&
+        (!min_max_arg_part || (min_max_arg_part - first_non_group_part > 0)))
+    {
+      if (tree)
+      {
+        uint dummy;
+        SEL_ARG *index_range_tree= get_index_range_tree(cur_index, tree, param,
+                                                        &dummy);
+        if (!get_constant_key_infix(cur_index_info, index_range_tree,
+                                    first_non_group_part, min_max_arg_part,
+                                    last_part, thd, cur_key_infix, 
+                                    &cur_key_infix_len,
+                                    &first_non_infix_part))
         {
-          /*
-            There is a gap but no range tree, thus no predicates at all for the
-            non-group keyparts.
-          */
-          cause= "no_nongroup_keypart_predicate";
+          cause= "nonconst_equality_gap_attribute";
           goto next_index;
         }
-        else if (first_non_group_part && join->conds)
+      }
+      else if (min_max_arg_part &&
+               (min_max_arg_part - first_non_group_part > 0))
+      {
+        /*
+          There is a gap but no range tree, thus no predicates at all for the
+          non-group keyparts.
+        */
+        cause= "no_nongroup_keypart_predicate";
+        goto next_index;
+      }
+      else if (first_non_group_part && join->conds)
+      {
+        /*
+          If there is no MIN/MAX function in the query, but some index
+          key part is referenced in the WHERE clause, then this index
+          cannot be used because the WHERE condition over the keypart's
+          field cannot be 'pushed' to the index (because there is no
+          range 'tree'), and the WHERE clause must be evaluated before
+          GROUP BY/DISTINCT.
+        */
+        /*
+          Store the first and last keyparts that need to be analyzed
+          into one array that can be passed as parameter.
+        */
+        KEY_PART_INFO *key_part_range[2];
+        key_part_range[0]= first_non_group_part;
+        key_part_range[1]= last_part;
+
+        /* Check if cur_part is referenced in the WHERE clause. */
+        if (join->conds->walk(&Item::find_item_in_field_list_processor, 0,
+                              (uchar*) key_part_range))
         {
-          /*
-            If there is no MIN/MAX function in the query, but some index
-            key part is referenced in the WHERE clause, then this index
-            cannot be used because the WHERE condition over the keypart's
-            field cannot be 'pushed' to the index (because there is no
-            range 'tree'), and the WHERE clause must be evaluated before
-            GROUP BY/DISTINCT.
-          */
-          /*
-            Store the first and last keyparts that need to be analyzed
-            into one array that can be passed as parameter.
-          */
-          KEY_PART_INFO *key_part_range[2];
-          key_part_range[0]= first_non_group_part;
-          key_part_range[1]= last_part;
-
-          /* Check if cur_part is referenced in the WHERE clause. */
-          if (join->conds->walk(&Item::find_item_in_field_list_processor, 0,
-                                (uchar*) key_part_range))
-          {
-            cause= "keypart_reference_from_where_clause";
-            goto next_index;
-          }
+          cause= "keypart_reference_from_where_clause";
+          goto next_index;
         }
       }
+    }
 
-      /*
-        Test (WA1) partially - that no other keypart after the last infix part is
-        referenced in the query.
-      */
-      if (first_non_infix_part)
+    /*
+      Test (WA1) partially - that no other keypart after the last infix part is
+      referenced in the query.
+    */
+    if (first_non_infix_part)
+    {
+      cur_part= first_non_infix_part +
+        (min_max_arg_part && (min_max_arg_part < last_part));
+      for (; cur_part != last_part; cur_part++)
       {
-        cur_part= first_non_infix_part +
-          (min_max_arg_part && (min_max_arg_part < last_part));
-        for (; cur_part != last_part; cur_part++)
+        if (bitmap_is_set(table->read_set, cur_part->field->field_index))
         {
-          if (bitmap_is_set(table->read_set, cur_part->field->field_index))
-          {
-            cause= "keypart_after_infix_in_query";
-            goto next_index;
-          }
+          cause= "keypart_after_infix_in_query";
+          goto next_index;
         }
       }
+    }
 
-      /* If we got to this point, cur_index_info passes the test. */
-      key_infix_parts= cur_key_infix_len ? (uint) 
-        (first_non_infix_part - first_non_group_part) : 0;
-      cur_used_key_parts= cur_group_key_parts + key_infix_parts;
-
-      /* Compute the cost of using this index. */
-      if (tree)
-      {
-        /* Find the SEL_ARG sub-tree that corresponds to the chosen index. */
-        cur_index_tree= get_index_range_tree(cur_index, tree, param,
-                                             &cur_param_idx);
-        /* Check if this range tree can be used for prefix retrieval. */
-        COST_VECT dummy_cost;
-        uint mrr_flags= HA_MRR_USE_DEFAULT_IMPL;
-        uint mrr_bufsize=0;
-        cur_quick_prefix_records= check_quick_select(param, cur_param_idx, 
-                                                     FALSE /*don't care*/, 
-                                                     cur_index_tree, TRUE,
-                                                     &mrr_flags, &mrr_bufsize,
-                                                     &dummy_cost);
-      }
-      cost_group_min_max(table, cur_index_info, cur_used_key_parts,
-                         cur_group_key_parts, tree, cur_index_tree,
-                         cur_quick_prefix_records, have_min, have_max,
-                         &cur_read_cost, &cur_records);
-      /*
-        If cur_read_cost is lower than best_read_cost use cur_index.
-        Do not compare doubles directly because they may have different
-        representations (64 vs. 80 bits).
-      */
-      trace_idx.add("records", cur_records).add("cost", cur_read_cost);
-      if (cur_read_cost < best_read_cost - (DBL_EPSILON * cur_read_cost))
-      {
-        index_info= cur_index_info;
-        index= cur_index;
-        best_read_cost= cur_read_cost;
-        best_records= cur_records;
-        best_index_tree= cur_index_tree;
-        best_quick_prefix_records= cur_quick_prefix_records;
-        best_param_idx= cur_param_idx;
-        group_key_parts= cur_group_key_parts;
-        group_prefix_len= cur_group_prefix_len;
-        key_infix_len= cur_key_infix_len;
-        if (key_infix_len)
-          memcpy (key_infix, cur_key_infix, sizeof (key_infix));
-        used_key_parts= cur_used_key_parts;
-      }
+    /* If we got to this point, cur_index_info passes the test. */
+    key_infix_parts= cur_key_infix_len ? (uint) 
+      (first_non_infix_part - first_non_group_part) : 0;
+    cur_used_key_parts= cur_group_key_parts + key_infix_parts;
+
+    /* Compute the cost of using this index. */
+    if (tree)
+    {
+      /* Find the SEL_ARG sub-tree that corresponds to the chosen index. */
+      cur_index_tree= get_index_range_tree(cur_index, tree, param,
+                                           &cur_param_idx);
+      /* Check if this range tree can be used for prefix retrieval. */
+      COST_VECT dummy_cost;
+      uint mrr_flags= HA_MRR_USE_DEFAULT_IMPL;
+      uint mrr_bufsize=0;
+      cur_quick_prefix_records= check_quick_select(param, cur_param_idx, 
+                                                   FALSE /*don't care*/, 
+                                                   cur_index_tree, TRUE,
+                                                   &mrr_flags, &mrr_bufsize,
+                                                   &dummy_cost);
+    }
+    cost_group_min_max(table, cur_index_info, cur_used_key_parts,
+                       cur_group_key_parts, tree, cur_index_tree,
+                       cur_quick_prefix_records, have_min, have_max,
+                       &cur_read_cost, &cur_records);
+    /*
+      If cur_read_cost is lower than best_read_cost use cur_index.
+      Do not compare doubles directly because they may have different
+      representations (64 vs. 80 bits).
+    */
+    trace_idx.add("records", cur_records).add("cost", cur_read_cost);
+    if (cur_read_cost < best_read_cost - (DBL_EPSILON * cur_read_cost))
+    {
+      index_info= cur_index_info;
+      index= cur_index;
+      best_read_cost= cur_read_cost;
+      best_records= cur_records;
+      best_index_tree= cur_index_tree;
+      best_quick_prefix_records= cur_quick_prefix_records;
+      best_param_idx= cur_param_idx;
+      group_key_parts= cur_group_key_parts;
+      group_prefix_len= cur_group_prefix_len;
+      key_infix_len= cur_key_infix_len;
+      if (key_infix_len)
+        memcpy (key_infix, cur_key_infix, sizeof (key_infix));
+      used_key_parts= cur_used_key_parts;
+    }
 
   next_index:
-      if (cause)
-      {
-        trace_idx.add("usable", false).add_alnum("cause", cause);
-        cause= NULL;
-      }
+    if (cause)
+    {
+      trace_idx.add("usable", false).add_alnum("cause", cause);
+      cause= NULL;
     }
   }
+  ota.end();
+    
+  
   if (!index_info) /* No usable index found. */
     DBUG_RETURN(NULL);
 
@@ -10535,7 +10540,8 @@ get_best_group_min_max(PARAM *param, SEL
                                       Field::itMBR : Field::itRAW))
   {
     cause= "unsupported_predicate_on_agg_attribute";
-    goto return_null_not_usable_cause;
+    trace_group.add("usable", false).add_alnum("cause", cause);
+    DBUG_RETURN(NULL);
   }
   /* The query passes all tests, so construct a new TRP object. */
   read_plan= new (param->mem_root)
@@ -10568,15 +10574,11 @@ get_best_group_min_max(PARAM *param, SEL
 
   DBUG_RETURN(read_plan);
 
-return_null_not_chosen_cause:
-  DBUG_ASSERT(cause);
-  trace_group.add("chosen", false).add_alnum("cause", cause);
-  DBUG_RETURN(NULL);
-
-return_null_not_usable_cause:
-  DBUG_ASSERT(cause);
-  trace_group.add("usable", false).add_alnum("cause", cause);
-  DBUG_RETURN(NULL);
+//return_null_not_chosen_cause:
+//DBUG_ASSERT(cause);
+
+//return_null_not_usable_cause:
+//DBUG_ASSERT(cause);
 }
 
 


Attachment: [text/bzr-bundle] bzr/tor.didriksen@oracle.com-20110112080009-1sn63ncpnnvqwb0e.bundle
Thread
bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3244) WL#5257Tor Didriksen12 Jan
  • Re: bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3244)WL#5257Guilhem Bichot12 Jan