Xavier (Open ERP) has proposed merging
lp:~openerp-dev/openobject-server/trunk-read-group-aggregated-fields-selection-improvements-xmo
into lp:openobject-server.
Requested reviews:
OpenERP Core Team (openerp)
For more details, see:
https://code.launchpad.net/~openerp-dev/openobject-server/trunk-read-group-aggregated-fields-selection-improvements-xmo/+merge/82272
Simplify and clarify the selection of fields to aggregate in read_group:
* rename `fields_pre` to `aggregated_fields`
* merge the `float_int_fields` filter (ensures that all aggregated fields are
integer or float fields) into the `aggregated_fields` listcomp (it's the only
place where the variable is used besides the `dict.fromkeys` which should be
using the actual aggregated fields list anyway)
* merge the exclusion of the `id` and `sequence` fields (from the following
iteration) into that listcomp as well, they're no use in the collection of
aggregated fields
This way there is no intermediate list built, and we create a list exactly as
long as we need.
A few questions remain, but they risk actually changing the behavior of the
method, so I did not touch them:
* why would the CONCURRENCY_CHECK_FIELD be aggregated? This seems to be the
result of a copy/paste from read_flat, there's very similar field line 3314:
fields_pre = [f for f in fields_to_read if
f == self.CONCURRENCY_CHECK_FIELD
or (f in self._columns and getattr(self._columns[f],
'_classic_write'))
] + self._inherits.values()
* is the `f in self._columns and getattr(self._columns[f], '_classic_write')`
part of the filter actually correct?
--
https://code.launchpad.net/~openerp-dev/openobject-server/trunk-read-group-aggregated-fields-selection-improvements-xmo/+merge/82272
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openobject-server/trunk-read-group-aggregated-fields-selection-improvements-xmo.
=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py 2011-11-15 11:45:27 +0000
+++ openerp/osv/orm.py 2011-11-15 12:33:34 +0000
@@ -2407,7 +2407,6 @@
# TODO it seems fields_get can be replaced by _all_columns (no need for translation)
fget = self.fields_get(cr, uid, fields)
- float_int_fields = filter(lambda x: fget[x]['type'] in ('float', 'integer'), fields)
flist = ''
group_count = group_by = groupby
if groupby:
@@ -2423,17 +2422,18 @@
raise except_orm(_('Invalid group_by'),
_('Invalid group_by specification: "%s".\nA group_by specification must be a list of valid fields.')%(groupby,))
-
- fields_pre = [f for f in float_int_fields if
- f == self.CONCURRENCY_CHECK_FIELD
- or (f in self._columns and getattr(self._columns[f], '_classic_write'))]
- for f in fields_pre:
- if f not in ['id', 'sequence']:
- group_operator = fget[f].get('group_operator', 'sum')
- if flist:
- flist += ', '
- qualified_field = '"%s"."%s"' % (self._table, f)
- flist += "%s(%s) AS %s" % (group_operator, qualified_field, f)
+ aggregated_fields = [
+ f for f in fields
+ if f not in ('id', 'sequence')
+ if fget[f]['type'] in ('integer', 'float')
+ if f == self.CONCURRENCY_CHECK_FIELD
+ or (f in self._columns and getattr(self._columns[f], '_classic_write'))]
+ for f in aggregated_fields:
+ group_operator = fget[f].get('group_operator', 'sum')
+ if flist:
+ flist += ', '
+ qualified_field = '"%s"."%s"' % (self._table, f)
+ flist += "%s(%s) AS %s" % (group_operator, qualified_field, f)
gb = groupby and (' GROUP BY ' + qualified_groupby_field) or ''
@@ -2486,7 +2486,7 @@
if (pos<len(data)) and (data[pos][groupby][0] == stages[pos][0]):
pos+=1
continue
- val = dict.fromkeys(float_int_fields, False)
+ val = dict.fromkeys(aggregated_fields, False)
val.update({
groupby: stages[pos],
'__domain': [(groupby, '=', stages[pos][0])]+domain,
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
