List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:April 1 2011 1:52pm
Subject:Re: bzr commit into mysql-trunk branch (jorgen.loland:3351) Bug#11882131
View as plain text  
2011/4/1 Jorgen Loland <jorgen.loland@stripped>

> #At file:///export/home/jl208045/mysql/mysql-trunk-11882131/ based on
> revid:mattias.jonsson@stripped
>
>  3351 Jorgen Loland     2011-04-01
>      BUG#11882131: OPTIMIZER CHOOSES FILESORT WHEN REVERSE
>                    INDEX SCAN COULD BE USED
>
>
LGTM
Some comments:

>> === modified file 'sql/opt_range.cc'
>> --- sql/opt_range.cc    2011-03-22 11:44:40 +0000
>> +++ sql/opt_range.cc    2011-04-01 13:02:51 +0000
>> @@ -742,6 +742,10 @@
>>    bool is_ror_scan;
>>    /* Number of ranges in the last checked tree->key */
>>    uint n_ranges;
>> +
>> +  /* Order direction
>> +*/
>> +  ORDER::enum_order order_direction;

That comment is a bit useless (just repeats the name of the class member),
and strangely formatted...


>> === modified file 'sql/opt_range.h'
>> --- sql/opt_range.h    2011-03-22 11:44:40 +0000
>> +++ sql/opt_range.h    2011-04-01 13:02:51 +0000
>> @@ -276,7 +276,15 @@
>>    /* Range end should be called when we have looped over the whole index
*/
>>    virtual void range_end() {}
>>
>> +  /**
>> +    Whether the range access method returns records in reverse order.
>> +  */
>>    virtual bool reverse_sorted() = 0;

I guess this function could/should be const as well.


>> === modified file 'sql/sql_select.cc'
>> --- sql/sql_select.cc    2011-03-29 07:20:17 +0000
>> +++ sql/sql_select.cc    2011-04-01 13:02:51 +0000
>> @@ -18424,9 +18430,12 @@
>>  {
>>    delete tab->select->quick;
>>    tab->select->quick=0;

How about a followup-patch, where lines like these are replaced with:
  tab->select->set_quick(NULL);



>> @@ -19651,9 +19660,11 @@
>>      if (key_part->field != field || !field->part_of_sortkey.is_set(idx))
>>        DBUG_RETURN(0);
>>
>> +    const ORDER::enum_order keypart_order=
>> +      (key_part->key_part_flag & HA_REVERSE_SORT) ?
>> +      ORDER::ORDER_DESC : ORDER::ORDER_ASC;
>>      /* set flag to 1 if we can use read-next on key, else to -1 */
>> -    flag= ((order->asc == !(key_part->key_part_flag &
> HA_REVERSE_SORT))
?
>> -           1 : -1);
>> +    flag= (order->direction == keypart_order) ? 1 : -1;

thank you :-)


>> === modified file 'sql/table.h'
>> --- sql/table.h    2011-03-30 11:43:32 +0000
>> +++ sql/table.h    2011-04-01 13:02:51 +0000
>> @@ -194,7 +194,13 @@
>>    Item     *item_ptr;            /* Storage for initial item */
>>    int    counter;                       /* position in SELECT list,
correct
>>                                             only if counter_used is
true*/
>> -  bool     asc;                /* true if ascending */
>> +  enum enum_order {
>> +    ORDER_NOT_RELEVANT,
>> +    ORDER_ASC,
>> +    ORDER_DESC
>> +  };
>> +
>> +  enum_order direction;                 /* Requested direction of
ordering */
>>    bool     free_me;            /* true if item isn't shared  */
>>    bool     in_field_list;            /* true if in select field list */
>>    bool   counter_used;                  /* parameter was counter of
columns */
>>

Nicely aligned, but could you un-tabify as well?




>
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:
> http://lists.mysql.com/commits?unsub=1
>

Thread
bzr commit into mysql-trunk branch (jorgen.loland:3351) Bug#11882131Jorgen Loland1 Apr
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3351) Bug#11882131Tor Didriksen1 Apr
    • Re: bzr commit into mysql-trunk branch (jorgen.loland:3351) Bug#11882131Jorgen Loland1 Apr
    • Re: bzr commit into mysql-trunk branch (jorgen.loland:3351) Bug#11882131Jorgen Loland6 Apr