From: Roy Lyseng
Date: April 29 2011 9:32am
Subject: Re: bzr commit into mysql-trunk branch (jorgen.loland:3654) Bug#11766327
List-Archive: http://lists.mysql.com/commits/136384
Message-Id: <4DBA85C3.1060208@oracle.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
 boundary="------------030709080908010307010400"

--------------030709080908010307010400
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Hi Jørgen,

this part of the bugfix is approved.

(Although I dislike that JOIN_TAB::set_condition() is used both in the 
optimization and the execution phase).

Thanks,
Roy

On 16.02.11 10.43, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-trunk-59415/ based on revid:jorgen.loland@stripped
>
>   3654 Jorgen Loland	2011-02-16
>        BUG#11766327: Range analysis should not be done many times
>                      for the same index
>
>        (Former bug 59415)
>
>        The optimizer often choses to access a table through an index
>        that does not provide the correct ordering for free. To remedy
>        this, the function test_if_skip_sort_order() is called to see
>        if another index is as good as the chosen index and at the
>        same time is able to provide ordering.
>
>        This implies that test_if_skip_sort_ordering() goes through a
>        full range analysis (if range access is applicable) to check
>        whether or not another range access plan should be used
>        instead of the currently chosen ref/range access method.
>
>        The problem is that if range analysis is performed and it is
>        decided that it is not better than whatever we had, the range
>        analysis will most likely be performed again and again with
>        the same outcome because test_if_skip_sort_order() is called
>        from multiple locations.
>
>        This patch avoids the unnecessarily repeated range analysis
>        described above by introducing key_map
>        JOIN_TAB::quick_order_tested which is checked to see
>        if range analysis has already been performed for a given key.
>       @ sql/sql_select.h
>          Introduce JOIN_TAB::quick_order_tested used to avoid repeated range analysis for the same key in test_if_skip_sort_order()
>
>      modified:
>        sql/sql_select.cc
>        sql/sql_select.h
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2011-02-16 09:27:55 +0000
> +++ b/sql/sql_select.cc	2011-02-16 09:43:49 +0000
> @@ -20096,8 +20096,11 @@ test_if_skip_sort_order(JOIN_TAB *tab,OR
>
>       if (best_key>= 0)
>       {
> -      if (table->quick_keys.is_set(best_key)&&  best_key != ref_key)
> +      if (table->quick_keys.is_set(best_key)&&
> +          !tab->quick_order_tested.is_set(best_key)&&
> +          best_key != ref_key)
>         {
> +        tab->quick_order_tested.set_bit(best_key);
>           key_map map;           // Force the creation of quick select
>           map.set_bit(best_key); // only best_key.
>           select->quick= 0;
>
> === modified file 'sql/sql_select.h'
> --- a/sql/sql_select.h	2011-02-16 09:27:55 +0000
> +++ b/sql/sql_select.h	2011-02-16 09:43:49 +0000
> @@ -281,6 +281,15 @@ public:
>     key_map	checked_keys;			/**<  Keys checked */
>     key_map	needed_reg;
>     key_map       keys;                           /**<  all keys with can be used */
> +  /**
> +    Used to avoid repeated range analysis for the same key in
> +    test_if_skip_sort_order(). This would otherwise happen if the best
> +    range access plan found for a key is turned down.
> +    quick_order_tested is cleared every time the select condition for
> +    this JOIN_TAB changes since a new condition may give another plan
> +    and cost from range analysis.
> +   */
> +  key_map       quick_order_tested;
>
>     /* Either #rows in the table or 1 for const table.  */
>     ha_rows	records;
> @@ -427,6 +436,7 @@ public:
>       DBUG_PRINT("info", ("JOIN_TAB::cond changes %p ->  %p at line %u tab %p",
>                           cond, to, line, this));
>       cond= to;
> +    quick_order_tested.clear_all();
>     }
>     Item *set_jt_and_sel_cond(Item *new_cond, uint line)
>     {
> @@ -470,6 +480,7 @@ st_join_table::st_join_table()
>       checked_keys(),
>       needed_reg(),
>       keys(),
> +    quick_order_tested(),
>
>       records(0),
>       found_records(0),
>
>
>
>


--------------030709080908010307010400--