From: Ole John Aske
Date: February 16 2011 12:55pm
Subject: Re: bzr commit into mysql-trunk branch (ole.john.aske:3549) Bug#59326
List-Archive: http://lists.mysql.com/commits/131416
Message-Id: <4D5BC950.9050109@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

On 02/16/11 10:34 AM, Olav Sandstaa wrote:

>> @@ -8262,6 +8264,7 @@ best_extension_by_limited_search(JOIN
>>
>> if ( (search_depth> 1)&& (remaining_tables& ~real_table_bit)& allowed_tables )
>> { /* Recursively expand the current partial plan */
>> + current_read_time+= current_record_count / (double) TIME_FOR_COMPARE;
>
> With this change this is the first code that is run both for the if-part and the else-part of this if-statement. I think
> it would be better to move this outside/before the if (and you might have already done so in one of the patches that I
> have not yet looked at).
> It might also be a good idea to add a comment to it saying why we add the CPU cost here and not earlier in the function
> where we add the IO cost.

This issue is taken care of in one of the other commits related to this bug.

   http://lists.mysql.com/commits/130055

3551 Ole John Aske	2011-01-31
       Refactoring cleanup related to bug#59326 and my previous set of fixes for this bug:

       - Added 'TIME_FOR_COMPARE' to current_read_time where
         it is initially calculated - Removed later adding of
         TIME_FOR_COMPARE whenever it is used.
.......

       This fix contains no (intentional) changes of logic.

For this changeset I have chosen to keep #lines changed as small as
possible in order to keep focus on only the logical changes
required in the greed optimizer.


>
>> swap_variables(JOIN_TAB*, join->best_ref[idx], *pos);
>> if (best_extension_by_limited_search(join,
>> remaining_tables& ~real_table_bit,
>> @@ -8288,7 +8291,7 @@ best_extension_by_limited_search(JOIN
>> Hence it may be wrong.
>> */
>> current_read_time+= current_record_count;
>> - if ((search_depth == 1) || (current_read_time< join->best_read))
>> + if (current_read_time< join->best_read)
>> {
>> memcpy((uchar*) join->best_positions, (uchar*) join->positions,
>> sizeof(POSITION) * (idx + 1));
>>
>>
>>
>>
>
>