Hi Jørgen,
On 04/01/2011 02:40 PM, Jorgen Loland wrote:
> Hi Olav,
>
> I have committed a new patch. All suggestions except the one below
> have been included.
The updated patch looks good.
>
>>> === modified file 'sql/sql_parse.cc'
>>> --- a/sql/sql_parse.cc 2011-03-08 17:39:25 +0000
>>> +++ b/sql/sql_parse.cc 2011-03-28 08:45:06 +0000
>>> @@ -5688,7 +5688,7 @@ bool add_to_list(THD *thd, SQL_I_List<OR
>>> DBUG_RETURN(1);
>>> order->item_ptr= item;
>>> order->item=&order->item_ptr;
>>> - order->asc = asc;
>>> + order->order = asc ? ORDER::ORDER_ASC : ORDER::ORDER_DESC;
>>
>> Minor suggestions:
>>
>> a. Remve the " " in front of the "="
>> b. Add ( ) around the "asc ? ... " (I think it makes it easier to
>> read.. - but
>> feel free to ignore this)
>> c. Did you consider changing the last argument to add_to_list() from
>> bool to
>> ORDER::enum_order?
>
> There are a gazillion of places that has "bool asc" or similar that
> could be changed to use the new enum. I've chosen to not explode the
> patch by fixing all these. It's on my todo list, but probably won't
> happen anytime soon :-(
That is fine. It was more of a question than a request for including
even more in the patch.
Olav
