From: Jorgen Loland
Date: April 1 2011 1:58pm
Subject: Re: bzr commit into mysql-trunk branch (jorgen.loland:3351) Bug#11882131
List-Archive: http://lists.mysql.com/commits/134474
Message-Id: <4D95DA18.5070908@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Tor,

thank you for the review.

>>> === 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...

Indeed. It turned out I had unsaved changes in this emacs buffer :-)

>>> === 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.

done

>>> === 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);

good suggestion


>>> === 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?

done

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway