[FIX] models: "ORDER BY" on many2one fields

When ordering results on a many2one fields, results are ordered by
order of the target model. The code was wrongly assuming that this
`_order` attribute only contains `_classic_read` fields (that can be
directly read from the table in database). Now correctly generate the
"ORDER BY" clause using the current table alias.

`res.users` can now be sorted by name.
This commit is contained in:
Christophe Simonis 2015-08-13 16:52:58 +02:00
parent d8c5299dc0
commit dac52e344c
4 changed files with 69 additions and 56 deletions

View File

@ -148,6 +148,7 @@ class res_users(osv.osv):
}
_name = "res.users"
_description = 'Users'
_order = 'name, login'
def _set_new_password(self, cr, uid, id, name, value, args, context=None):
if value is False:

View File

@ -104,6 +104,21 @@ class test_search(common.TransactionCase):
self.assertEqual(test_user_ids, expected_ids, 'search on res_users did not provide expected ids or expected order')
users_obj._order = old_order
def test_11_indirect_inherits_m2order(self):
registry, cr, uid = self.registry, self.cr, self.uid
Cron = registry('ir.cron')
Users = registry('res.users')
user_ids = {}
cron_ids = {}
for u in 'BAC':
user_ids[u] = Users.create(cr, uid, {'name': u, 'login': u})
cron_ids[u] = Cron.create(cr, uid, {'name': u, 'user_id': user_ids[u]})
ids = Cron.search(cr, uid, [('id', 'in', cron_ids.values())], order='user_id')
expected_ids = [cron_ids[l] for l in 'ABC']
self.assertEqual(ids, expected_ids)
if __name__ == '__main__':
unittest2.main()

View File

@ -1946,7 +1946,7 @@ class BaseModel(object):
gb_function = split[1] if len(split) == 2 else None
temporal = field_type in ('date', 'datetime')
tz_convert = field_type == 'datetime' and context.get('tz') in pytz.all_timezones
qualified_field = self._inherits_join_calc(split[0], query)
qualified_field = self._inherits_join_calc(self._table, split[0], query)
if temporal:
display_formats = {
# Careful with week/year formats:
@ -2103,7 +2103,7 @@ class BaseModel(object):
if getattr(self._fields[f].base_field.column, '_classic_write', False)
]
field_formatter = lambda f: (self._fields[f].group_operator or 'sum', self._inherits_join_calc(f, query), f)
field_formatter = lambda f: (self._fields[f].group_operator or 'sum', self._inherits_join_calc(self._table, f, query), f)
select_terms = ["%s(%s) AS %s" % field_formatter(f) for f in aggregated_fields]
for gb in annotated_groupbys:
@ -2177,17 +2177,18 @@ class BaseModel(object):
parent_alias, parent_alias_statement = query.add_join((current_model._table, parent_model._table, inherits_field, 'id', inherits_field), implicit=True)
return parent_alias
def _inherits_join_calc(self, field, query):
def _inherits_join_calc(self, alias, field, query):
"""
Adds missing table select and join clause(s) to ``query`` for reaching
the field coming from an '_inherits' parent table (no duplicates).
:param alias: name of the initial SQL alias
:param field: name of inherited field to reach
:param query: query object on which the JOIN should be added
:return: qualified name of field, to be used in SELECT clause
"""
# INVARIANT: alias is the SQL alias of model._table in query
model, alias = self, self._table
model = self
while field in model._inherit_fields and field not in model._columns:
# retrieve the parent model where field is inherited from
parent_model_name = model._inherit_fields[field][0]
@ -3274,7 +3275,7 @@ class BaseModel(object):
def qualify(field):
col = field.name
if field.inherited:
res = self._inherits_join_calc(field.name, query)
res = self._inherits_join_calc(self._table, field.name, query)
else:
res = '"%s"."%s"' % (self._table, col)
if field.type == 'binary' and (context.get('bin_size') or context.get('bin_size_' + col)):
@ -4523,9 +4524,9 @@ class BaseModel(object):
for inherited_model in self._inherits:
rule_where_clause, rule_where_clause_params, rule_tables = rule_obj.domain_get(cr, uid, inherited_model, mode, context=context)
apply_rule(rule_where_clause, rule_where_clause_params, rule_tables,
parent_model=inherited_model)
parent_model=inherited_model)
def _generate_m2o_order_by(self, order_field, query):
def _generate_m2o_order_by(self, alias, order_field, query):
"""
Add possibly missing JOIN to ``query`` and generate the ORDER BY clause for m2o fields,
either native m2o fields or function/related fields that are stored, including
@ -4535,10 +4536,10 @@ class BaseModel(object):
"""
if order_field not in self._columns and order_field in self._inherit_fields:
# also add missing joins for reaching the table containing the m2o field
qualified_field = self._inherits_join_calc(order_field, query)
qualified_field = self._inherits_join_calc(alias, order_field, query)
order_field_column = self._inherit_fields[order_field][2]
else:
qualified_field = '"%s"."%s"' % (self._table, order_field)
qualified_field = '"%s"."%s"' % (alias, order_field)
order_field_column = self._columns[order_field]
assert order_field_column._type == 'many2one', 'Invalid field passed to _generate_m2o_order_by()'
@ -4554,19 +4555,52 @@ class BaseModel(object):
if not regex_order.match(m2o_order):
# _order is complex, can't use it here, so we default to _rec_name
m2o_order = dest_model._rec_name
else:
# extract the field names, to be able to qualify them and add desc/asc
m2o_order_list = []
for order_part in m2o_order.split(","):
m2o_order_list.append(order_part.strip().split(" ", 1)[0].strip())
m2o_order = m2o_order_list
# Join the dest m2o table if it's not joined yet. We use [LEFT] OUTER join here
# as we don't want to exclude results that have NULL values for the m2o
src_table, src_field = qualified_field.replace('"', '').split('.', 1)
dst_alias, dst_alias_statement = query.add_join((src_table, dest_model._table, src_field, 'id', src_field), implicit=False, outer=True)
qualify = lambda field: '"%s"."%s"' % (dst_alias, field)
return map(qualify, m2o_order) if isinstance(m2o_order, list) else qualify(m2o_order)
return dest_model._generate_order_by_inner(dst_alias, m2o_order, query)
def _generate_order_by_inner(self, alias, order_spec, query):
order_by_elements = []
self._check_qorder(order_spec)
for order_part in order_spec.split(','):
order_split = order_part.strip().split(' ')
order_field = order_split[0].strip()
order_direction = order_split[1].strip() if len(order_split) == 2 else ''
order_column = None
inner_clause = None
if order_field == 'id':
order_by_elements.append('"%s"."%s" %s' % (alias, order_field, order_direction))
elif order_field in self._columns:
order_column = self._columns[order_field]
if order_column._classic_read:
inner_clause = '"%s"."%s"' % (alias, order_field)
elif order_column._type == 'many2one':
inner_clause = self._generate_m2o_order_by(alias, order_field, query)
else:
continue # ignore non-readable or "non-joinable" fields
elif order_field in self._inherit_fields:
parent_obj = self.pool[self._inherit_fields[order_field][3]]
order_column = parent_obj._columns[order_field]
if order_column._classic_read:
inner_clause = self._inherits_join_calc(alias, order_field, query)
elif order_column._type == 'many2one':
inner_clause = self._generate_m2o_order_by(alias, order_field, query)
else:
continue # ignore non-readable or "non-joinable" fields
else:
raise ValueError(_("Sorting field %s not found on model %s") % (order_field, self._name))
if order_column and order_column._type == 'boolean':
inner_clause = "COALESCE(%s, false)" % inner_clause
if inner_clause:
if isinstance(inner_clause, list):
for clause in inner_clause:
order_by_elements.append("%s %s" % (clause, order_direction))
else:
order_by_elements.append("%s %s" % (inner_clause, order_direction))
return order_by_elements
def _generate_order_by(self, order_spec, query):
"""
@ -4578,43 +4612,7 @@ class BaseModel(object):
order_by_clause = ''
order_spec = order_spec or self._order
if order_spec:
order_by_elements = []
self._check_qorder(order_spec)
for order_part in order_spec.split(','):
order_split = order_part.strip().split(' ')
order_field = order_split[0].strip()
order_direction = order_split[1].strip() if len(order_split) == 2 else ''
order_column = None
inner_clause = None
if order_field == 'id':
order_by_elements.append('"%s"."%s" %s' % (self._table, order_field, order_direction))
elif order_field in self._columns:
order_column = self._columns[order_field]
if order_column._classic_read:
inner_clause = '"%s"."%s"' % (self._table, order_field)
elif order_column._type == 'many2one':
inner_clause = self._generate_m2o_order_by(order_field, query)
else:
continue # ignore non-readable or "non-joinable" fields
elif order_field in self._inherit_fields:
parent_obj = self.pool[self._inherit_fields[order_field][3]]
order_column = parent_obj._columns[order_field]
if order_column._classic_read:
inner_clause = self._inherits_join_calc(order_field, query)
elif order_column._type == 'many2one':
inner_clause = self._generate_m2o_order_by(order_field, query)
else:
continue # ignore non-readable or "non-joinable" fields
else:
raise ValueError( _("Sorting field %s not found on model %s") %( order_field, self._name))
if order_column and order_column._type == 'boolean':
inner_clause = "COALESCE(%s, false)" % inner_clause
if inner_clause:
if isinstance(inner_clause, list):
for clause in inner_clause:
order_by_elements.append("%s %s" % (clause, order_direction))
else:
order_by_elements.append("%s %s" % (inner_clause, order_direction))
order_by_elements = self._generate_order_by_inner(self._table, order_spec, query)
if order_by_elements:
order_by_clause = ",".join(order_by_elements)

View File

@ -988,12 +988,11 @@ class many2many(_column):
wquery = obj._where_calc(cr, user, domain, context=context)
obj._apply_ir_rules(cr, user, wquery, 'read', context=context)
order_by = obj._generate_order_by(None, wquery)
from_c, where_c, where_params = wquery.get_sql()
if where_c:
where_c = ' AND ' + where_c
order_by = ' ORDER BY "%s".%s' %(obj._table, obj._order.split(',')[0])
limit_str = ''
if self._limit is not None:
limit_str = ' LIMIT %d' % self._limit