[IMP] Speed and quality improvement

- removing the need of the use of search when groupby is set on a relation
    field.
  - creation and use of dedicated helper method to compute the orderby clause
    for easier reading

bzr revid: acl@openerp.com-20140217140144-2rm3o00g76tyhqxn

openerp/osv/orm.py

@@ -2599,6 +2599,32 @@ class BaseModel(object):
                 r['__fold'] = folded.get(r[groupby] and r[groupby][0], False)
         return result
 
+    def _read_group_generate_order_by(self, orderby, aggregated_fields, groupby, query):
+        """
+        Generates the ORDER BY sql clause for the read group method. Adds the missing JOIN clause
+        to the query if order should be computed against m2o field. 
+        :param orderby: the orderby definition in the form "%(field)s %(order)s"
+        :param aggregated_fields: list of aggregated fields in the query
+        :param groupby: the current groupby field name
+        :param query: the query object used to construct the query afterwards
+        """
+        orderby_list = []
+        ob = ''
+        for order_splits in orderby.split(','):
+            order_split = order_splits.split()
+            orderby_field = order_split[0]
+            orderby_dir = len(order_split) == 2 and order_split[1].upper() == 'ASC' and 'ASC' or 'DESC'
+            if orderby_field == groupby:
+                ob = self._generate_order_by(order_splits, query).replace('ORDER BY ', '')
+                orderby_list.append(ob)
+            elif orderby_field in aggregated_fields:
+                orderby_list.append('%s %s' % (orderby_field,orderby_dir))
+                
+        if orderby_list:
+            return ' ORDER BY %s' % (','.join(orderby_list)), ob
+        else:
+            return '', ob
+
     def read_group(self, cr, uid, domain, fields, groupby, offset=0, limit=None, context=None, orderby=False):
         """
         Get the list of records in list view grouped by the given ``groupby`` fields
@@ -2678,15 +2704,11 @@ class BaseModel(object):
             qualified_field = '"%s"."%s"' % (self._table, f)
             flist += "%s(%s) AS %s" % (group_operator, qualified_field, f)
 
-        orderby_clause = ""
-        if orderby:
-            orderby_list = []
-            order_splits = [o.strip().split(' ') for o in orderby.split(',')]
-            for order_split in order_splits:
-                if order_split[0] in aggregated_fields or order_split[0] == groupby:
-                    orderby_list += [(order_split[0], (len(order_split) == 2 and order_split[1].strip() == 'DESC') and 'DESC' or 'ASC')]
-            if orderby_list:
-                orderby_clause = 'ORDER BY %s' % (','.join([ '%s %s' % (order[0], order[1]) for order in orderby_list]))
+        order = orderby or groupby
+        orderby_clause = ''
+        ob = ''
+        if order:
+            orderby_clause, ob = self._read_group_generate_order_by(order, aggregated_fields, groupby, query)
 
         gb = groupby and (' GROUP BY ' + qualified_groupby_field) or ''
 
@@ -2696,27 +2718,21 @@ class BaseModel(object):
         offset_str = offset and ' offset %d' % offset or ''
         if len(groupby_list) < 2 and context.get('group_by_no_leaf'):
             group_count = '_'
-        cr.execute('SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + orderby_clause + limit_str + offset_str, where_clause_params)
+        query = 'SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist  + ' FROM ' + from_clause + where_clause + gb + (ob and ',') + ob  + orderby_clause + limit_str + offset_str
+        cr.execute(query , where_clause_params)
         alldata = {}
         groupby = group_by
 
         fetched_data = cr.dictfetchall()
 
-        # we need to keep order of previous search to properly order data on aggregated fields
-        order = orderby or groupby
-        data_ids = [data['id'] for data in fetched_data]
-
+        data_ids = []
         for r in fetched_data:
             for fld, val in r.items():
                 if val is None: r[fld] = False
             alldata[r['id']] = r
+            data_ids.append(r['id'])
             del r['id']
 
-        if order and order.split(',')[0].replace(' ASC', '').replace(' DESC', '') not in aggregated_fields:
-            data_ids = self.search(cr, uid, [('id', 'in', data_ids)], order=order, context=context)
-            # the IDs of records that have groupby field value = False or '' should be included too
-            data_ids += set(alldata.keys()).difference(data_ids)
-
         if groupby:
             data = self.read(cr, uid, data_ids, [groupby], context=context)
             # restore order of the search as read() uses the default _order (this is only for groups, so the footprint of data should be small):