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
>