From 266cad9a2eabf10dccd2b7ca11571b7f41e6705f Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Fri, 22 Jul 2011 10:10:30 +0200 Subject: [PATCH 01/24] [IMP] unaccent: added an unaccent command-line flag, and a function to test unaccent availability. bzr revid: vmt@openerp.com-20110722081030-ltxuqknpe1arduw5 --- openerp/modules/db.py | 10 ++++++++++ openerp/modules/registry.py | 5 +++++ openerp/tools/config.py | 2 ++ 3 files changed, 17 insertions(+) diff --git a/openerp/modules/db.py b/openerp/modules/db.py index 25ce7283c97..2e61212925c 100644 --- a/openerp/modules/db.py +++ b/openerp/modules/db.py @@ -118,4 +118,14 @@ def create_categories(cr, categories): categories = categories[1:] return p_id +def has_unaccent(cr): + """ Test if the database has an unaccent function. + + The unaccent is supposed to be provided by the PostgreSQL unaccent contrib + module but any similar function will be picked by OpenERP. + + """ + cr.execute("SELECT proname FROM pg_proc WHERE proname='unaccent'") + return len(cr.fetchall()) > 0 + # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: diff --git a/openerp/modules/registry.py b/openerp/modules/registry.py index 11f484a71a0..bc80a7bc354 100644 --- a/openerp/modules/registry.py +++ b/openerp/modules/registry.py @@ -25,6 +25,7 @@ import openerp.sql_db import openerp.osv.orm +import openerp.modules.db class Registry(object): @@ -44,6 +45,10 @@ class Registry(object): self.db_name = db_name self.db = openerp.sql_db.db_connect(db_name) + cr = self.db.cursor() + self.has_unaccent = openerp.modules.db.has_unaccent(cr) + cr.close() + def do_parent_store(self, cr): for o in self._init_parent: self.get(o)._parent_store_compute(cr) diff --git a/openerp/tools/config.py b/openerp/tools/config.py index 18f40e0011f..b09ef040cea 100644 --- a/openerp/tools/config.py +++ b/openerp/tools/config.py @@ -250,6 +250,8 @@ class configmanager(object): "osv_memory tables. This is a decimal value expressed in hours, " "and the default is 1 hour.", type="float") + group.add_option("--unaccent", dest="unaccent", my_default=False, + help="Use the unaccent function provided by the database when available.") parser.add_option_group(group) # Copy all optparse options (i.e. MyOption) into self.options. From a9e89a49b270a21bbd31535eaccdcbafa66480b1 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Fri, 22 Jul 2011 17:36:49 +0200 Subject: [PATCH 02/24] [REF] expression: instanciating an expression now "parses" it, moved some methods as functions, introduced some nice globals. bzr revid: vmt@openerp.com-20110722153649-atojff0vp34076id --- .../addons/base/test/test_osv_expression.yml | 24 +++ openerp/osv/expression.py | 175 +++++++++--------- openerp/osv/orm.py | 6 +- 3 files changed, 111 insertions(+), 94 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 3a5e41dacc7..b01a5cf3770 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -177,6 +177,30 @@ res_ids = self.search(cr, uid, [('company_id.partner_id', 'not in', [])]) res_ids.sort() assert res_ids == all_ids, "Searching against empty set failed, returns %r" % res_ids +- + Equivalent queries. +- + !python {model: res.partner }: | + all_ids = self.search(cr, uid, []) + if len(all_ids) > 1: + one = all_ids[0] + record = self.browse(cr, uid, one) + others = all_ids[1:] + res_1 = self.search(cr, uid, [('id', '=', one)]) + # self.search(cr, uid, [('id', '!=', others)]) # not permitted + res_2 = self.search(cr, uid, [('id', 'not in', others)]) + res_3 = self.search(cr, uid, ['!', ('id', '!=', one)]) + res_4 = self.search(cr, uid, ['!', ('id', 'in', others)]) + # res_5 = self.search(cr, uid, [('id', 'in', one)]) # TODO make it permitted, just like for child_of + res_6 = self.search(cr, uid, [('id', 'in', [one])]) + res_7 = self.search(cr, uid, [('name', '=', record.name)]) + assert [one] == res_1 + assert [one] == res_2 + assert [one] == res_3 + assert [one] == res_4 + #assert [one] == res_5 + assert [one] == res_6 + assert [one] == res_7 - Verify that normalize_domain() works. - diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 54726a289f2..dab2e0ad79a 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -29,8 +29,11 @@ NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' -TRUE_DOMAIN = [(1,'=',1)] -FALSE_DOMAIN = [(0,'=',1)] +TRUE_LEAF = (1, '=', 1) +FALSE_LEAF = (0, '=', 1) + +TRUE_DOMAIN = [TRUE_LEAF] +FALSE_DOMAIN = [FALSE_LEAF] def normalize(domain): """Returns a normalized version of ``domain_expr``, where all implicit '&' operators @@ -91,6 +94,39 @@ def OR(domains): """ OR([D1,D2,...]) returns a domain representing D1 or D2 or ... """ return combine(OR_OPERATOR, FALSE_DOMAIN, TRUE_DOMAIN, domains) +def is_operator(element): + return isinstance(element, (str, unicode)) and element in [AND_OPERATOR, OR_OPERATOR, NOT_OPERATOR] + +# TODO change the share wizard to use this function. +def is_leaf(element, internal=False): + OPS = ('=', '!=', '<>', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') + INTERNAL_OPS = OPS + ('inselect',) + return (isinstance(element, tuple) or isinstance(element, list)) \ + and len(element) == 3 \ + and (((not internal) and element[1] in OPS) \ + or (internal and element[1] in INTERNAL_OPS)) + +def execute_recursive_in(cr, s, f, w, ids, op): + # todo: merge into parent query as sub-query + res = [] + if ids: + if op in ['<','>','>=','<=']: + cr.execute('SELECT "%s"' \ + ' FROM "%s"' \ + ' WHERE "%s" %s %%s' % (s, f, w, op), (ids[0],)) + res.extend([r[0] for r in cr.fetchall()]) + else: + for i in range(0, len(ids), cr.IN_MAX): + subids = ids[i:i+cr.IN_MAX] + cr.execute('SELECT "%s"' \ + ' FROM "%s"' \ + ' WHERE "%s" IN %%s' % (s, f, w),(tuple(subids),)) + res.extend([r[0] for r in cr.fetchall()]) + else: + cr.execute('SELECT distinct("%s")' \ + ' FROM "%s" where "%s" is not null' % (s, f, s)), + res.extend([r[0] for r in cr.fetchall()]) + return res class expression(object): """ @@ -100,62 +136,27 @@ class expression(object): For more info: http://christophe-simonis-at-tiny.blogspot.com/2008/08/new-new-domain-notation.html """ - @classmethod - def _is_operator(cls, element): - return isinstance(element, (str, unicode)) and element in [AND_OPERATOR, OR_OPERATOR, NOT_OPERATOR] - - @classmethod - def _is_leaf(cls, element, internal=False): - OPS = ('=', '!=', '<>', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') - INTERNAL_OPS = OPS + ('inselect',) - return (isinstance(element, tuple) or isinstance(element, list)) \ - and len(element) == 3 \ - and (((not internal) and element[1] in OPS) \ - or (internal and element[1] in INTERNAL_OPS)) - - def __execute_recursive_in(self, cr, s, f, w, ids, op, type): - # todo: merge into parent query as sub-query - res = [] - if ids: - if op in ['<','>','>=','<=']: - cr.execute('SELECT "%s"' \ - ' FROM "%s"' \ - ' WHERE "%s" %s %%s' % (s, f, w, op), (ids[0],)) - res.extend([r[0] for r in cr.fetchall()]) - else: - for i in range(0, len(ids), cr.IN_MAX): - subids = ids[i:i+cr.IN_MAX] - cr.execute('SELECT "%s"' \ - ' FROM "%s"' \ - ' WHERE "%s" IN %%s' % (s, f, w),(tuple(subids),)) - res.extend([r[0] for r in cr.fetchall()]) - else: - cr.execute('SELECT distinct("%s")' \ - ' FROM "%s" where "%s" is not null' % (s, f, s)), - res.extend([r[0] for r in cr.fetchall()]) - return res - - def __init__(self, exp): + def __init__(self, cr, uid, exp, table, context): # check if the expression is valid - if not reduce(lambda acc, val: acc and (self._is_operator(val) or self._is_leaf(val)), exp, True): + if not reduce(lambda acc, val: acc and (is_operator(val) or is_leaf(val)), exp, True): raise ValueError('Bad domain expression: %r' % (exp,)) - self.__exp = exp self.__field_tables = {} # used to store the table to use for the sql generation. key = index of the leaf self.__all_tables = set() self.__joins = [] self.__main_table = None # 'root' table. set by parse() - self.__DUMMY_LEAF = (1, '=', 1) # a dummy leaf that must not be parsed or sql generated + # assign self.__exp with the normalized, parsed domain. + self.parse(cr, uid, normalize(exp), table, context) @property def exp(self): return self.__exp[:] - def parse(self, cr, uid, table, context): + def parse(self, cr, uid, exp, table, context): """ transform the leafs of the expression """ - if not self.__exp: - return self + self.__exp = exp - def _rec_get(ids, table, parent=None, left='id', prefix=''): + def _rec_get(right, table, parent=None, left='id', prefix=''): + ids = child_of_right_to_ids(right, 'ilike') if table._parent_store and (not table.pool._init): # TODO: Improve where joins are implemented for many with '.', replace by: # doms += ['&',(prefix+'.parent_left','<',o.parent_right),(prefix+'.parent_left','>=',o.parent_left)] @@ -175,15 +176,12 @@ class expression(object): return ids + rg(ids2, table, parent) return [(left, 'in', rg(ids, table, parent or table._parent_name))] - def child_of_right_to_ids(value): + # TODO rename this function as it is not strictly for 'child_of', but also for 'in'... + def child_of_right_to_ids(value, operator): """ Normalize a single id, or a string, or a list of ids to a list of ids. - - This function is always used with _rec_get() above, so it should be - called directly from _rec_get instead of repeatedly before _rec_get. - """ if isinstance(value, basestring): - return [x[0] for x in field_obj.name_search(cr, uid, value, [], 'ilike', context=context, limit=None)] + return [x[0] for x in field_obj.name_search(cr, uid, value, [], operator, context=context, limit=None)] elif isinstance(value, (int, long)): return [value] else: @@ -196,7 +194,7 @@ class expression(object): while i + 1','!=']: o2m_op = 'not in' - self.__exp[i] = ('id', o2m_op, self.__execute_recursive_in(cr, field._fields_id, field_obj._table, 'id', ids2, operator, field._type)) + self.__exp[i] = ('id', o2m_op, execute_recursive_in(cr, field._fields_id, field_obj._table, 'id', ids2, operator)) if call_null: o2m_op = 'not in' if operator in ['not like','not ilike','not in','<>','!=']: o2m_op = 'in' - self.__exp[i] = ('id', o2m_op, self.__execute_recursive_in(cr, field._fields_id, field_obj._table, 'id', [], operator, field._type) or [0]) + self.__exp[i] = ('id', o2m_op, execute_recursive_in(cr, field._fields_id, field_obj._table, 'id', [], operator) or [0]) elif field._type == 'many2many': #FIXME @@ -313,10 +308,9 @@ class expression(object): def _rec_convert(ids): if field_obj == table: return ids - return self.__execute_recursive_in(cr, field._id1, field._rel, field._id2, ids, operator, field._type) + return execute_recursive_in(cr, field._id1, field._rel, field._id2, ids, operator) - ids2 = child_of_right_to_ids(right) - dom = _rec_get(ids2, field_obj) + dom = _rec_get(right, field_obj) ids2 = field_obj.search(cr, uid, dom, context=context) self.__exp[i] = ('id', 'in', _rec_convert(ids2)) else: @@ -335,7 +329,7 @@ class expression(object): if operator in ['like','ilike','in','=']: #no result found with given search criteria call_null_m2m = False - self.__exp[i] = ('id','=',0) + self.__exp[i] = FALSE_LEAF else: call_null_m2m = True operator = 'in' # operator changed because ids are directly related to main object @@ -345,21 +339,19 @@ class expression(object): if operator in ['not like','not ilike','not in','<>','!=']: m2m_op = 'not in' - self.__exp[i] = ('id', m2m_op, self.__execute_recursive_in(cr, field._id1, field._rel, field._id2, res_ids, operator, field._type) or [0]) + self.__exp[i] = ('id', m2m_op, execute_recursive_in(cr, field._id1, field._rel, field._id2, res_ids, operator) or [0]) if call_null_m2m: m2m_op = 'not in' if operator in ['not like','not ilike','not in','<>','!=']: m2m_op = 'in' - self.__exp[i] = ('id', m2m_op, self.__execute_recursive_in(cr, field._id1, field._rel, field._id2, [], operator, field._type) or [0]) + self.__exp[i] = ('id', m2m_op, execute_recursive_in(cr, field._id1, field._rel, field._id2, [], operator) or [0]) elif field._type == 'many2one': if operator == 'child_of': - ids2 = child_of_right_to_ids(right) - self.__operator = 'in' if field._obj != working_table._name: - dom = _rec_get(ids2, field_obj, left=left, prefix=field._obj) + dom = _rec_get(right, field_obj, left=left, prefix=field._obj) else: - dom = _rec_get(ids2, working_table, parent=left) + dom = _rec_get(right, working_table, parent=left) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] else: def _get_expression(field_obj,cr, uid, left, right, operator, context=None): @@ -379,7 +371,7 @@ class expression(object): operator = dict_op[operator] res_ids = field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c) if not res_ids: - return ('id','=',0) + return FALSE_LEAF else: right = map(lambda x: x[0], res_ids) return (left, 'in', right) @@ -398,9 +390,9 @@ class expression(object): m2o_str = False if operator in ('not in', '!=', '<>'): # (many2one not in []) should return all records - self.__exp[i] = self.__DUMMY_LEAF + self.__exp[i] = TRUE_LEAF else: - self.__exp[i] = ('id','=',0) + self.__exp[i] = FALSE_LEAF else: new_op = '=' if operator in ['not like','not ilike','not in','<>','!=']: @@ -459,11 +451,12 @@ class expression(object): ] self.__exp[i] = ('id', 'inselect', (query1, query2)) - return self def __leaf_to_sql(self, leaf, table): - if leaf == self.__DUMMY_LEAF: - return ('(1=1)', []) + if leaf == TRUE_LEAF: + return ('TRUE', []) + if leaf == FALSE_LEAF: + return ('FALSE', []) left, operator, right = leaf if operator == 'inselect': @@ -478,7 +471,7 @@ class expression(object): len_after = len(params) check_nulls = len_after != len_before - query = '(1=0)' + query = 'FALSE' if len_after: if left == 'id': @@ -508,7 +501,7 @@ class expression(object): elif (operator == '=?'): op = '=' if (right is False or right is None): - return ( 'TRUE',[]) + return ('TRUE', []) if left in table._columns: format = table._columns[left]._symbol_set[0] query = '(%s.%s %s %s)' % (table._table, left, op, format) @@ -545,7 +538,7 @@ class expression(object): params = table._columns[left]._symbol_set[1](right) if add_null: - query = '(%s OR %s IS NULL)' % (query, left) + query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) if isinstance(params, basestring): params = [params] @@ -555,8 +548,9 @@ class expression(object): def to_sql(self): stack = [] params = [] + # Process the domain from right to left, using a stack, to generate a SQL expression. for i, e in reverse_enumerate(self.__exp): - if self._is_leaf(e, internal=True): + if is_leaf(e, internal=True): table = self.__field_tables.get(i, self.__main_table) q, p = self.__leaf_to_sql(e, table) params.insert(0, p) @@ -570,10 +564,11 @@ class expression(object): q2 = stack.pop() stack.append('(%s %s %s)' % (q1, ops[e], q2,)) - query = ' AND '.join(reversed(stack)) + assert len(stack) == 1 + query = stack[0] joins = ' AND '.join(self.__joins) if joins: - query = '(%s) AND (%s)' % (joins, query) + query = '(%s) AND %s' % (joins, query) return (query, flatten(params)) def get_tables(self): diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 48d66f30e32..034042533ae 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -2410,8 +2410,7 @@ class orm_memory(orm_template): args = [('active', '=', 1)] if args: import expression - e = expression.expression(args) - e.parse(cr, user, self, context) + e = expression.expression(cr, user, args, self, context) res = e.exp return res or [] @@ -4351,8 +4350,7 @@ class orm(orm_template): if domain: import expression - e = expression.expression(domain) - e.parse(cr, user, self, context) + e = expression.expression(cr, user, domain, self, context) tables = e.get_tables() where_clause, where_params = e.to_sql() where_clause = where_clause and [where_clause] or [] From 5858a06d5d3a93bf582dfaf3f3aee3ff4da34f6e Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Mon, 25 Jul 2011 17:07:21 +0200 Subject: [PATCH 03/24] [IMP] expression: some tests, some cleaning, and a fix: child_of_right_to_ids() had an uninitialized field_obj variable. bzr revid: vmt@openerp.com-20110725150721-mm4fhnhm17ill29b --- .../addons/base/test/test_osv_expression.yml | 49 ++++++++++++++++++- openerp/osv/expression.py | 49 +++++++++++-------- 2 files changed, 77 insertions(+), 21 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index b01a5cf3770..59e1c93f2c9 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -178,7 +178,7 @@ res_ids.sort() assert res_ids == all_ids, "Searching against empty set failed, returns %r" % res_ids - - Equivalent queries. + Equivalent queries, integer and string. - !python {model: res.partner }: | all_ids = self.search(cr, uid, []) @@ -194,6 +194,8 @@ # res_5 = self.search(cr, uid, [('id', 'in', one)]) # TODO make it permitted, just like for child_of res_6 = self.search(cr, uid, [('id', 'in', [one])]) res_7 = self.search(cr, uid, [('name', '=', record.name)]) + res_8 = self.search(cr, uid, [('name', 'in', [record.name])]) + # res_9 = self.search(cr, uid, [('name', 'in', record.name)]) # TODO assert [one] == res_1 assert [one] == res_2 assert [one] == res_3 @@ -201,6 +203,51 @@ #assert [one] == res_5 assert [one] == res_6 assert [one] == res_7 +- + Need a company with a parent_id. +- + !record {model: res.company, id: ymltest_company3}: + name: Acme 3 +- + Need a company with a parent_id. +- + !record {model: res.company, id: ymltest_company4}: + name: Acme 4 + parent_id: ymltest_company3 +- + Equivalent queries, one2many. +- + !python {model: res.company }: | + # Search the company via its one2many (the one2many must point back at the company). + company = self.browse(cr, uid, ref('ymltest_company3')) + default_company = self.browse(cr, uid, 1) + # one2many towards same model + res_1 = self.search(cr, uid, [('child_ids', 'in', [x.id for x in company.child_ids])]) + res_2 = self.search(cr, uid, [('child_ids', 'in', [company.child_ids[0].id])]) + # one2many towards another model + res_3 = self.search(cr, uid, [('currency_ids', 'in', [x.id for x in default_company.currency_ids])]) + res_4 = self.search(cr, uid, [('currency_ids', 'in', [default_company.currency_ids[0].id])]) + assert res_1 == [ref('ymltest_company3')] + assert res_2 == [ref('ymltest_company3')] + assert res_3 == [1] + assert res_4 == [1] + + # child_of x returns x and its children (direct or not). + company = self.browse(cr, uid, ref('ymltest_company3')) + expected = [ref('ymltest_company3'), ref('ymltest_company4')] + expected.sort() + res_1 = self.search(cr, uid, [('id', 'child_of', [ref('ymltest_company3')])]) + res_1.sort() + res_2 = self.search(cr, uid, [('id', 'child_of', ref('ymltest_company3'))]) + res_2.sort() + #res_3 = self.search(cr, uid, [('id', 'child_of', [company.name])]) + #res_3.sort() + res_4 = self.search(cr, uid, [('id', 'child_of', company.name)]) + res_4.sort() + assert res_1 == expected + assert res_2 == expected + #assert res_3 == expected + assert res_4 == expected - Verify that normalize_domain() works. - diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index dab2e0ad79a..b94f12dec67 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -147,6 +147,7 @@ class expression(object): # assign self.__exp with the normalized, parsed domain. self.parse(cr, uid, normalize(exp), table, context) + # TODO used only for osv_memory @property def exp(self): return self.__exp[:] @@ -155,8 +156,8 @@ class expression(object): """ transform the leafs of the expression """ self.__exp = exp - def _rec_get(right, table, parent=None, left='id', prefix=''): - ids = child_of_right_to_ids(right, 'ilike') + def child_of_domain(left, right, table, parent=None, prefix=''): + ids = right if table._parent_store and (not table.pool._init): # TODO: Improve where joins are implemented for many with '.', replace by: # doms += ['&',(prefix+'.parent_left','<',o.parent_right),(prefix+'.parent_left','>=',o.parent_left)] @@ -177,7 +178,7 @@ class expression(object): return [(left, 'in', rg(ids, table, parent or table._parent_name))] # TODO rename this function as it is not strictly for 'child_of', but also for 'in'... - def child_of_right_to_ids(value, operator): + def child_of_right_to_ids(value, operator, field_obj): """ Normalize a single id, or a string, or a list of ids to a list of ids. """ if isinstance(value, basestring): @@ -198,25 +199,30 @@ class expression(object): continue left, operator, right = e operator = operator.lower() - working_table = table - main_table = table + working_table = table # The table containing the field (the name provided in the left operand) fargs = left.split('.', 1) + + # If the field is _inherits'd, search for the working_table, + # and extract the field. if fargs[0] in table._inherit_fields: while True: - field = main_table._columns.get(fargs[0]) + field = working_table._columns.get(fargs[0]) if field: self.__field_tables[i] = working_table break - working_table = main_table.pool.get(main_table._inherit_fields[fargs[0]][0]) - if working_table not in self.__all_tables: - self.__joins.append('%s.%s=%s.%s' % (working_table._table, 'id', main_table._table, main_table._inherits[working_table._name])) - self.__all_tables.add(working_table) - main_table = working_table + next_table = working_table.pool.get(working_table._inherit_fields[fargs[0]][0]) + if next_table not in self.__all_tables: + self.__joins.append('%s.%s=%s.%s' % (next_table._table, 'id', working_table._table, working_table._inherits[next_table._name])) + self.__all_tables.add(next_table) + working_table = next_table + # Or (try to) directly extract the field. + else: + field = working_table._columns.get(fargs[0]) - field = working_table._columns.get(fargs[0]) if not field: if left == 'id' and operator == 'child_of': - dom = _rec_get(right, working_table) + ids2 = child_of_right_to_ids(right, 'ilike', table) + dom = child_of_domain(left, ids2, working_table) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] continue @@ -263,9 +269,11 @@ class expression(object): # Applying recursivity on field(one2many) if operator == 'child_of': if field._obj != working_table._name: - dom = _rec_get(right, field_obj, left=left, prefix=field._obj) + ids2 = child_of_right_to_ids(right, 'ilike', field_obj) + dom = child_of_domain(left, ids2, field_obj, prefix=field._obj) else: - dom = _rec_get(right, working_table, parent=left) + ids2 = child_of_right_to_ids(right, 'ilike', field_obj) + dom = child_of_domain('id', ids2, working_table, parent=left) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] else: @@ -287,7 +295,6 @@ class expression(object): call_null = False self.__exp[i] = FALSE_LEAF else: - call_null = True operator = 'in' # operator changed because ids are directly related to main object else: call_null = False @@ -310,7 +317,8 @@ class expression(object): return ids return execute_recursive_in(cr, field._id1, field._rel, field._id2, ids, operator) - dom = _rec_get(right, field_obj) + ids2 = child_of_right_to_ids(right, 'ilike', field_obj) + dom = child_of_domain('id', ids2, field_obj) ids2 = field_obj.search(cr, uid, dom, context=context) self.__exp[i] = ('id', 'in', _rec_convert(ids2)) else: @@ -331,7 +339,6 @@ class expression(object): call_null_m2m = False self.__exp[i] = FALSE_LEAF else: - call_null_m2m = True operator = 'in' # operator changed because ids are directly related to main object else: call_null_m2m = False @@ -349,9 +356,11 @@ class expression(object): elif field._type == 'many2one': if operator == 'child_of': if field._obj != working_table._name: - dom = _rec_get(right, field_obj, left=left, prefix=field._obj) + ids2 = child_of_right_to_ids(right, 'ilike', field_obj) + dom = child_of_domain(left, ids2, field_obj, prefix=field._obj) else: - dom = _rec_get(right, working_table, parent=left) + ids2 = child_of_right_to_ids(right, 'ilike', field_obj) + dom = child_of_domain('id', ids2, working_table, parent=left) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] else: def _get_expression(field_obj,cr, uid, left, right, operator, context=None): From 7b8dfa72ed50f874ca9f28a7ef1367473600f0b9 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 26 Jul 2011 14:56:05 +0200 Subject: [PATCH 04/24] [IMP] expression: added some tests. bzr revid: vmt@openerp.com-20110726125605-jr1fi2su315nkanz --- .../addons/base/test/test_osv_expression.yml | 50 ++++++++++++++++++- openerp/osv/expression.py | 10 ++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 59e1c93f2c9..a372c5952a3 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -177,6 +177,25 @@ res_ids = self.search(cr, uid, [('company_id.partner_id', 'not in', [])]) res_ids.sort() assert res_ids == all_ids, "Searching against empty set failed, returns %r" % res_ids +- + Equivalent queries. +- + !python {model: res.currency }: | + max_currency_id = max(self.search(cr, uid, [])) + res_0 = self.search(cr, uid, []) + res_1 = self.search(cr, uid, [('name', 'not like', 'probably_unexisting_name')]) + res_2 = self.search(cr, uid, [('id', 'not in', [max_currency_id + 1003])]) + res_3 = self.search(cr, uid, [('id', 'not in', [])]) + res_4 = self.search(cr, uid, [('id', 'not in', False)]) + res_0.sort() + res_1.sort() + res_2.sort() + res_3.sort() + res_4.sort() + assert res_0 == res_1 + assert res_0 == res_2 + assert res_0 == res_3 + assert res_0 == res_4 - Equivalent queries, integer and string. - @@ -220,6 +239,10 @@ !python {model: res.company }: | # Search the company via its one2many (the one2many must point back at the company). company = self.browse(cr, uid, ref('ymltest_company3')) + max_currency_id = max(self.pool.get('res.currency').search(cr, uid, [])) + currency_ids = self.pool.get('res.currency').search(cr, uid, [('name', 'not like', 'probably_unexisting_name')]) + currency_ids = self.pool.get('res.currency').search(cr, uid, [('id', 'not in', [max_currency_id + 1003])]) + currency_ids = self.pool.get('res.currency').search(cr, uid, [('id', 'not in', [])]) default_company = self.browse(cr, uid, 1) # one2many towards same model res_1 = self.search(cr, uid, [('child_ids', 'in', [x.id for x in company.child_ids])]) @@ -227,10 +250,35 @@ # one2many towards another model res_3 = self.search(cr, uid, [('currency_ids', 'in', [x.id for x in default_company.currency_ids])]) res_4 = self.search(cr, uid, [('currency_ids', 'in', [default_company.currency_ids[0].id])]) + res_5 = self.search(cr, uid, [('currency_ids', 'in', default_company.currency_ids[0].id)]) + # res_6 = self.search(cr, uid, [('currency_ids', 'in', [default_company.currency_ids[0].name])]) # TODO + res_7 = self.search(cr, uid, [('currency_ids', '=', default_company.currency_ids[0].name)]) + res_8 = self.search(cr, uid, [('currency_ids', 'like', default_company.currency_ids[0].name)]) + res_9 = self.search(cr, uid, [('currency_ids', 'like', 'probably_unexisting_name')]) + # self.search(cr, uid, [('currency_ids', 'unexisting_op', 'probably_unexisting_name')]) # TODO expected exception + res_10 = self.search(cr, uid, [('currency_ids', 'not like', 'probably_unexisting_name')]) + res_11 = self.search(cr, uid, [('currency_ids', 'not in', [max_currency_id + 1])]) + res_12 = self.search(cr, uid, [('currency_ids', 'not in', False)]) + res_13 = self.search(cr, uid, [('currency_ids', 'not in', [])]) + res_10.sort() + res_11.sort() + res_12.sort() + res_13.sort() assert res_1 == [ref('ymltest_company3')] assert res_2 == [ref('ymltest_company3')] assert res_3 == [1] assert res_4 == [1] + assert res_5 == [1] + assert res_7 == [1] + assert res_8 == [1] + assert res_9 == [] + print ">>> 10:", res_10 + print ">>> 11:", res_11 + print ">>> 12:", res_12 + print ">>> 13:", res_13 + #assert res_10 == res_11 + #assert res_10 == res_12 + #assert res_10 == res_13 # child_of x returns x and its children (direct or not). company = self.browse(cr, uid, ref('ymltest_company3')) @@ -240,7 +288,7 @@ res_1.sort() res_2 = self.search(cr, uid, [('id', 'child_of', ref('ymltest_company3'))]) res_2.sort() - #res_3 = self.search(cr, uid, [('id', 'child_of', [company.name])]) + #res_3 = self.search(cr, uid, [('id', 'child_of', [company.name])]) # TODO #res_3.sort() res_4 = self.search(cr, uid, [('id', 'child_of', company.name)]) res_4.sort() diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index b94f12dec67..f623c7519c8 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -138,8 +138,9 @@ class expression(object): def __init__(self, cr, uid, exp, table, context): # check if the expression is valid - if not reduce(lambda acc, val: acc and (is_operator(val) or is_leaf(val)), exp, True): - raise ValueError('Bad domain expression: %r' % (exp,)) + for x in exp: + if not (is_operator(x) or is_leaf(x)): + raise ValueError('Bad domain expression: %r, %r is not a valid operator or a valid term.' % (exp, x)) self.__field_tables = {} # used to store the table to use for the sql generation. key = index of the leaf self.__all_tables = set() self.__joins = [] @@ -378,12 +379,11 @@ class expression(object): operator = dict_op[operator] elif isinstance(right,list) and operator in ['<>','!=','=']: #for domain (FIELD,'=',['value1','value2']) operator = dict_op[operator] - res_ids = field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c) + res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c)] if not res_ids: return FALSE_LEAF else: - right = map(lambda x: x[0], res_ids) - return (left, 'in', right) + return (left, 'in', res_ids) m2o_str = False if right: From 53ff3e9a13c8af7ebd15e8e2d9e398fb551da78e Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 26 Jul 2011 16:22:44 +0200 Subject: [PATCH 05/24] [REF] expression: splitted execute_recursive_in() in two: - it does two unrelated things - splitting it makes visible that ids2 and operator are no more used in some calls - _rec_convert now behaves differently but it probably cannot be called with an empty ids arg anyway. bzr revid: vmt@openerp.com-20110726142244-54ty1gn7jxxz3cwi --- openerp/osv/expression.py | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index f623c7519c8..1b387174857 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -106,28 +106,26 @@ def is_leaf(element, internal=False): and (((not internal) and element[1] in OPS) \ or (internal and element[1] in INTERNAL_OPS)) -def execute_recursive_in(cr, s, f, w, ids, op): +def select_from_where(cr, s, f, w, ids, op): # todo: merge into parent query as sub-query res = [] if ids: if op in ['<','>','>=','<=']: - cr.execute('SELECT "%s"' \ - ' FROM "%s"' \ - ' WHERE "%s" %s %%s' % (s, f, w, op), (ids[0],)) - res.extend([r[0] for r in cr.fetchall()]) - else: + cr.execute('SELECT "%s" FROM "%s" WHERE "%s" %s %%s' % \ + (s, f, w, op), (ids[0],)) # TODO shouldn't this be min/max(ids) ? + res = [r[0] for r in cr.fetchall()] + else: # TODO op is supposed to be 'in'? It is called with child_of... for i in range(0, len(ids), cr.IN_MAX): subids = ids[i:i+cr.IN_MAX] - cr.execute('SELECT "%s"' \ - ' FROM "%s"' \ - ' WHERE "%s" IN %%s' % (s, f, w),(tuple(subids),)) + cr.execute('SELECT "%s" FROM "%s" WHERE "%s" IN %%s' % \ + (s, f, w),(tuple(subids),)) res.extend([r[0] for r in cr.fetchall()]) - else: - cr.execute('SELECT distinct("%s")' \ - ' FROM "%s" where "%s" is not null' % (s, f, s)), - res.extend([r[0] for r in cr.fetchall()]) return res +def select_distinct_from_where_not_null(cr, s, f): + cr.execute('SELECT distinct("%s") FROM "%s" where "%s" is not null' % (s, f, s)) + return [r[0] for r in cr.fetchall()] + class expression(object): """ parse a domain expression @@ -302,13 +300,13 @@ class expression(object): o2m_op = 'in' if operator in ['not like','not ilike','not in','<>','!=']: o2m_op = 'not in' - self.__exp[i] = ('id', o2m_op, execute_recursive_in(cr, field._fields_id, field_obj._table, 'id', ids2, operator)) + self.__exp[i] = ('id', o2m_op, select_from_where(cr, field._fields_id, field_obj._table, 'id', ids2, operator)) if call_null: o2m_op = 'not in' if operator in ['not like','not ilike','not in','<>','!=']: o2m_op = 'in' - self.__exp[i] = ('id', o2m_op, execute_recursive_in(cr, field._fields_id, field_obj._table, 'id', [], operator) or [0]) + self.__exp[i] = ('id', o2m_op, select_distinct_from_where_not_null(cr, field._fields_id, field_obj._table) or [0]) elif field._type == 'many2many': #FIXME @@ -316,7 +314,7 @@ class expression(object): def _rec_convert(ids): if field_obj == table: return ids - return execute_recursive_in(cr, field._id1, field._rel, field._id2, ids, operator) + return select_from_where(cr, field._id1, field._rel, field._id2, ids, operator) ids2 = child_of_right_to_ids(right, 'ilike', field_obj) dom = child_of_domain('id', ids2, field_obj) @@ -346,13 +344,13 @@ class expression(object): m2m_op = 'in' if operator in ['not like','not ilike','not in','<>','!=']: m2m_op = 'not in' + self.__exp[i] = ('id', m2m_op, select_from_where(cr, field._id1, field._rel, field._id2, res_ids, operator) or [0]) - self.__exp[i] = ('id', m2m_op, execute_recursive_in(cr, field._id1, field._rel, field._id2, res_ids, operator) or [0]) if call_null_m2m: m2m_op = 'not in' if operator in ['not like','not ilike','not in','<>','!=']: m2m_op = 'in' - self.__exp[i] = ('id', m2m_op, execute_recursive_in(cr, field._id1, field._rel, field._id2, [], operator) or [0]) + self.__exp[i] = ('id', m2m_op, select_distinct_from_where_not_null(cr, field._id1, field._rel) or [0]) elif field._type == 'many2one': if operator == 'child_of': From 8522366247e5e081146a3de6be49803b1f7d298b Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 26 Jul 2011 17:21:22 +0200 Subject: [PATCH 06/24] [FIX] hack to (not) evaluate normalized domains: expression.py now always normalize its given domain, but the evaluation code for the orm_memory is just lacking. The fix simply skips any operator. bzr revid: vmt@openerp.com-20110726152122-hi8y65fixcx65me8 --- openerp/osv/orm.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 034042533ae..ffb3fb3a546 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -2440,6 +2440,9 @@ class orm_memory(orm_template): break f = True for arg in result: + if len(arg) != 3: + # Amazing hack: orm_memory handles only simple domains. + continue if arg[1] == '=': val = eval('data[arg[0]]'+'==' +' arg[2]', locals()) elif arg[1] in ['<', '>', 'in', 'not in', '<=', '>=', '<>']: From 705cbcf2130498394db2e16f1d53027121de2c6f Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 26 Jul 2011 17:24:35 +0200 Subject: [PATCH 07/24] [IMP] expression: make one more assert pass in expression.py tests. bzr revid: vmt@openerp.com-20110726152435-41jv1oryjkmiiy07 --- .../addons/base/test/test_osv_expression.yml | 23 +++++++++++++++---- openerp/osv/expression.py | 2 -- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index a372c5952a3..bad5ad0f611 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -177,6 +177,23 @@ res_ids = self.search(cr, uid, [('company_id.partner_id', 'not in', [])]) res_ids.sort() assert res_ids == all_ids, "Searching against empty set failed, returns %r" % res_ids +- + Property of the query (one2many not in False). +- + !python {model: res.currency }: | + ids = self.search(cr, uid, []) + referenced_companies = set([x.company_id.id for x in self.browse(cr, uid, ids)]) + companies = set(self.pool.get('res.company').search(cr, uid, [('currency_ids', 'not in', False)])) + assert referenced_companies == companies +- + Property of the query (one2many in False). +- + !python {model: res.currency }: | + ids = self.search(cr, uid, []) + referenced_companies = set([x.company_id.id for x in self.browse(cr, uid, ids)]) + unreferenced_companies = set(self.pool.get('res.company').search(cr, uid, [])).difference(referenced_companies) + companies = set(self.pool.get('res.company').search(cr, uid, [('currency_ids', 'in', False)])) + assert unreferenced_companies == companies - Equivalent queries. - @@ -274,11 +291,9 @@ assert res_9 == [] print ">>> 10:", res_10 print ">>> 11:", res_11 - print ">>> 12:", res_12 - print ">>> 13:", res_13 #assert res_10 == res_11 - #assert res_10 == res_12 - #assert res_10 == res_13 + assert res_10 == res_12 + assert res_10 == res_13 # child_of x returns x and its children (direct or not). company = self.browse(cr, uid, ref('ymltest_company3')) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 1b387174857..c645b35589f 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -293,8 +293,6 @@ class expression(object): #no result found with given search criteria call_null = False self.__exp[i] = FALSE_LEAF - else: - operator = 'in' # operator changed because ids are directly related to main object else: call_null = False o2m_op = 'in' From 1fc22273aa04f6393e0ceb7288d78d9d54d960ed Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 26 Jul 2011 19:24:58 +0200 Subject: [PATCH 08/24] [REF] cosmetic change. bzr revid: vmt@openerp.com-20110726172458-tg9pxfr29suhczgt --- openerp/osv/expression.py | 54 ++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index c645b35589f..5c20d92713a 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -51,7 +51,7 @@ def normalize(domain): result[0:0] = ['&'] # put an extra '&' in front expected = 1 result.append(token) - if isinstance(token, (list,tuple)): # domain term + if isinstance(token, (list, tuple)): # domain term expected -= 1 else: expected += op_arity.get(token, 0) - 1 @@ -118,7 +118,7 @@ def select_from_where(cr, s, f, w, ids, op): for i in range(0, len(ids), cr.IN_MAX): subids = ids[i:i+cr.IN_MAX] cr.execute('SELECT "%s" FROM "%s" WHERE "%s" IN %%s' % \ - (s, f, w),(tuple(subids),)) + (s, f, w), (tuple(subids),)) res.extend([r[0] for r in cr.fetchall()]) return res @@ -234,15 +234,15 @@ class expression(object): else: self.__exp[i] = (fargs[0], 'in', right) # Making search easier when there is a left operand as field.o2m or field.m2m - if field._type in ['many2many','one2many']: + if field._type in ['many2many', 'one2many']: right = field_obj.search(cr, uid, [(fargs[1], operator, right)], context=context) - right1 = table.search(cr, uid, [(fargs[0],'in', right)], context=context) + right1 = table.search(cr, uid, [(fargs[0], 'in', right)], context=context) if right1 == []: self.__exp[i] = FALSE_LEAF else: self.__exp[i] = ('id', 'in', right1) - if not isinstance(field,fields.property): + if not isinstance(field, fields.property): continue if field._properties and not field.store: @@ -284,7 +284,7 @@ class expression(object): if ids2: operator = 'in' else: - if not isinstance(right,list): + if not isinstance(right, list): ids2 = [right] else: ids2 = right @@ -360,7 +360,7 @@ class expression(object): dom = child_of_domain('id', ids2, working_table, parent=left) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] else: - def _get_expression(field_obj,cr, uid, left, right, operator, context=None): + def _get_expression(field_obj, cr, uid, left, right, operator, context=None): if context is None: context = {} c = context.copy() @@ -369,11 +369,11 @@ class expression(object): operator = ( operator in ['<','>','<=','>='] ) and 'in' or operator dict_op = {'not in':'!=','in':'=','=':'in','!=':'not in','<>':'not in'} - if isinstance(right,tuple): + if isinstance(right, tuple): right = list(right) - if (not isinstance(right,list)) and operator in ['not in','in']: + if (not isinstance(right, list)) and operator in ['not in','in']: operator = dict_op[operator] - elif isinstance(right,list) and operator in ['<>','!=','=']: #for domain (FIELD,'=',['value1','value2']) + elif isinstance(right, list) and operator in ['<>','!=','=']: #for domain (FIELD,'=',['value1','value2']) operator = dict_op[operator] res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c)] if not res_ids: @@ -385,7 +385,7 @@ class expression(object): if right: if isinstance(right, basestring): # and not isinstance(field, fields.related): m2o_str = True - elif isinstance(right,(list,tuple)): + elif isinstance(right, (list, tuple)): m2o_str = True for ele in right: if not isinstance(ele, basestring): @@ -403,10 +403,10 @@ class expression(object): if operator in ['not like','not ilike','not in','<>','!=']: new_op = '!=' #Is it ok to put 'left' and not 'id' ? - self.__exp[i] = (left,new_op,False) + self.__exp[i] = (left, new_op, False) if m2o_str: - self.__exp[i] = _get_expression(field_obj,cr, uid, left, right, operator, context=context) + self.__exp[i] = _get_expression(field_obj, cr, uid, left, right, operator, context=context) else: # other field type # add the time part to datetime field when it's not there: @@ -458,15 +458,20 @@ class expression(object): self.__exp[i] = ('id', 'inselect', (query1, query2)) def __leaf_to_sql(self, leaf, table): - if leaf == TRUE_LEAF: - return ('TRUE', []) - if leaf == FALSE_LEAF: - return ('FALSE', []) left, operator, right = leaf - if operator == 'inselect': + if leaf == TRUE_LEAF: + query = 'TRUE' + params = [] + + elif leaf == FALSE_LEAF: + query = 'FALSE' + params = [] + + elif operator == 'inselect': query = '(%s.%s in (%s))' % (table._table, left, right[0]) params = right[1] + elif operator in ['in', 'not in']: params = right and right[:] or [] len_before = len(params) @@ -478,6 +483,8 @@ class expression(object): check_nulls = len_after != len_before query = 'FALSE' + # TODO this code seems broken: 'not in [False]' will become 'true', + # i.e. 'not null or null', while I expect it to be 'not null'. if len_after: if left == 'id': instr = ','.join(['%s'] * len_after) @@ -492,15 +499,16 @@ class expression(object): query = '(%s.%s IS NOT NULL)' % (table._table, left) if check_nulls: query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) + else: params = [] - if right == False and (leaf[0] in table._columns) and table._columns[leaf[0]]._type=="boolean" and (operator == '='): - query = '(%s.%s IS NULL or %s.%s = false )' % (table._table, left,table._table, left) + if right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator == '='): + query = '(%s.%s IS NULL or %s.%s = false )' % (table._table, left, table._table, left) elif (((right == False) and (type(right)==bool)) or (right is None)) and (operator == '='): query = '%s.%s IS NULL ' % (table._table, left) - elif right == False and (leaf[0] in table._columns) and table._columns[leaf[0]]._type=="boolean" and (operator in ['<>', '!=']): - query = '(%s.%s IS NOT NULL and %s.%s != false)' % (table._table, left,table._table, left) + elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator in ['<>', '!=']): + query = '(%s.%s IS NOT NULL and %s.%s != false)' % (table._table, left, table._table, left) elif (((right == False) and (type(right)==bool)) or right is None) and (operator in ['<>', '!=']): query = '%s.%s IS NOT NULL' % (table._table, left) elif (operator == '=?'): @@ -522,7 +530,7 @@ class expression(object): else: like = operator in ('like', 'ilike', 'not like', 'not ilike') - op = {'=like':'like','=ilike':'ilike'}.get(operator,operator) + op = {'=like':'like','=ilike':'ilike'}.get(operator, operator) if left in table._columns: format = like and '%s' or table._columns[left]._symbol_set[0] query = '(%s.%s %s %s)' % (table._table, left, op, format) From 73151814122c88e1287aa32cece84a6911960bde Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 26 Jul 2011 19:28:53 +0200 Subject: [PATCH 09/24] [IMP] expression: added a small comment. bzr revid: vmt@openerp.com-20110726172853-4yoxl1bdnd1sopvf --- openerp/osv/expression.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 5c20d92713a..f3be51c7b50 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -60,7 +60,8 @@ def normalize(domain): def combine(operator, unit, zero, domains): """Returns a new domain expression where all domain components from ``domains`` - have been added together using the binary operator ``operator``. + have been added together using the binary operator ``operator``. The given + domains must be normalized. :param unit: the identity element of the domains "set" with regard to the operation performed by ``operator``, i.e the domain component ``i`` which, when @@ -72,6 +73,7 @@ def combine(operator, unit, zero, domains): combined with any domain ``x`` via ``operator``, yields ``z``. E.g. [(1,'=',1)] is the typical zero for OR_OPERATOR: as soon as you see it in a domain component the resulting domain is the zero. + :param domains: a list of normalized domains. """ result = [] count = 0 From 69eb0570639b753b04a0b20480fe4c59fcc67998 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 26 Jul 2011 20:48:06 +0200 Subject: [PATCH 10/24] [REF] expression: cosmetic changes. bzr revid: vmt@openerp.com-20110726184806-j3nejlzo9ez5epb7 --- openerp/osv/expression.py | 97 ++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index f3be51c7b50..a75c826b3d8 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -502,58 +502,63 @@ class expression(object): if check_nulls: query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) - else: + elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator == '='): + query = '(%s.%s IS NULL or %s.%s = false )' % (table._table, left, table._table, left) params = [] - if right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator == '='): - query = '(%s.%s IS NULL or %s.%s = false )' % (table._table, left, table._table, left) - elif (((right == False) and (type(right)==bool)) or (right is None)) and (operator == '='): - query = '%s.%s IS NULL ' % (table._table, left) - elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator in ['<>', '!=']): - query = '(%s.%s IS NOT NULL and %s.%s != false)' % (table._table, left, table._table, left) - elif (((right == False) and (type(right)==bool)) or right is None) and (operator in ['<>', '!=']): - query = '%s.%s IS NOT NULL' % (table._table, left) - elif (operator == '=?'): - op = '=' - if (right is False or right is None): - return ('TRUE', []) - if left in table._columns: - format = table._columns[left]._symbol_set[0] - query = '(%s.%s %s %s)' % (table._table, left, op, format) - params = table._columns[left]._symbol_set[1](right) - else: - query = "(%s.%s %s '%%s')" % (table._table, left, op) - params = right + elif (((right == False) and (type(right)==bool)) or (right is None)) and (operator == '='): + query = '%s.%s IS NULL ' % (table._table, left) + params = [] + elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator in ['<>', '!=']): + query = '(%s.%s IS NOT NULL and %s.%s != false)' % (table._table, left, table._table, left) + params = [] + + elif (((right == False) and (type(right)==bool)) or right is None) and (operator in ['<>', '!=']): + query = '%s.%s IS NOT NULL' % (table._table, left) + params = [] + + elif (operator == '=?'): + if (right is False or right is None): + query = 'TRUE' + params = [] + elif left in table._columns: + format = table._columns[left]._symbol_set[0] + query = '(%s.%s = %s)' % (table._table, left, format) + params = table._columns[left]._symbol_set[1](right) else: - if left == 'id': - query = '%s.id %s %%s' % (table._table, operator) - params = right + query = "(%s.%s = '%%s')" % (table._table, left) + params = right + + elif left == 'id': + query = '%s.id %s %%s' % (table._table, operator) + params = right + + else: + like = operator in ('like', 'ilike', 'not like', 'not ilike') + + op = {'=like':'like','=ilike':'ilike'}.get(operator, operator) + if left in table._columns: + format = like and '%s' or table._columns[left]._symbol_set[0] + query = '(%s.%s %s %s)' % (table._table, left, op, format) + else: + query = "(%s.%s %s '%s')" % (table._table, left, op, right) + + add_null = False + if like: + if isinstance(right, str): + str_utf8 = right + elif isinstance(right, unicode): + str_utf8 = right.encode('utf-8') else: - like = operator in ('like', 'ilike', 'not like', 'not ilike') + str_utf8 = str(right) + params = '%%%s%%' % str_utf8 + add_null = not str_utf8 + elif left in table._columns: + params = table._columns[left]._symbol_set[1](right) - op = {'=like':'like','=ilike':'ilike'}.get(operator, operator) - if left in table._columns: - format = like and '%s' or table._columns[left]._symbol_set[0] - query = '(%s.%s %s %s)' % (table._table, left, op, format) - else: - query = "(%s.%s %s '%s')" % (table._table, left, op, right) - - add_null = False - if like: - if isinstance(right, str): - str_utf8 = right - elif isinstance(right, unicode): - str_utf8 = right.encode('utf-8') - else: - str_utf8 = str(right) - params = '%%%s%%' % str_utf8 - add_null = not str_utf8 - elif left in table._columns: - params = table._columns[left]._symbol_set[1](right) - - if add_null: - query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) + if add_null: + query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) if isinstance(params, basestring): params = [params] From 2f09ebf7f4d233db24f2a1c999f6b822ee6964a0 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 9 Aug 2011 17:12:00 +0200 Subject: [PATCH 11/24] [IMP] expression: started some new tests. bzr revid: vmt@openerp.com-20110809151200-q9l2w7imt2uywciq --- .../addons/base/test/test_osv_expression.yml | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index bad5ad0f611..433b979da31 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -177,6 +177,57 @@ res_ids = self.search(cr, uid, [('company_id.partner_id', 'not in', [])]) res_ids.sort() assert res_ids == all_ids, "Searching against empty set failed, returns %r" % res_ids +- + Test the '(not) like/in' behavior. res.partner and its parent_id column are used because + parent_id is a many2one, allowing to test the Null value, and there are actually some + null and non-null values in the demo data. +- + !python {model: res.partner }: | + partner_ids = self.search(cr, uid, []) + max_partner_id = max(partner_ids) + partners = self.browse(cr, uid, partner_ids) + with_parent = [] + without_parent = [] + for x in partners: + if x.parent_id and x.parent_id.id in partner_ids: + with_parent.append(x.id) + for x in partners: + if not x.parent_id: + without_parent.append(x.id) + print "with_parent", with_parent + print "without_parent", without_parent + res_0 = self.search(cr, uid, [('parent_id', 'not like', 'probably_unexisting_name')]) + res_1 = self.search(cr, uid, [('parent_id', 'not in', [max_partner_id + 1])]) + res_2 = self.search(cr, uid, [('parent_id', 'not in', False)]) + res_3 = self.search(cr, uid, [('parent_id', 'not in', [])]) + res_4 = self.search(cr, uid, [('parent_id', 'not in', [False])]) + print ">>> 0:", res_0 + print ">>> 1:", res_1 + print ">>> 2:", res_2 + print ">>> 3:", res_3 + print ">>> 4:", res_4 + print ">>> ----------" + res_5 = self.search(cr, uid, [('parent_id', 'like', 'probably_unexisting_name')]) + res_6 = self.search(cr, uid, [('parent_id', 'in', [max_partner_id + 1])]) + res_7 = self.search(cr, uid, [('parent_id', 'in', False)]) + res_8 = self.search(cr, uid, [('parent_id', 'in', [])]) + res_9 = self.search(cr, uid, [('parent_id', 'in', [False])]) + print ">>> 5:", res_5 + print ">>> 6:", res_6 + print ">>> 7:", res_7 + print ">>> 8:", res_8 + print ">>> 9:", res_9 + print ">>> ----------" + res_10 = self.search(cr, uid, ['!', ('parent_id', 'like', 'probably_unexisting_name')]) + res_11 = self.search(cr, uid, ['!', ('parent_id', 'in', [max_partner_id + 1])]) + res_12 = self.search(cr, uid, ['!', ('parent_id', 'in', False)]) + res_13 = self.search(cr, uid, ['!', ('parent_id', 'in', [])]) + res_14 = self.search(cr, uid, ['!', ('parent_id', 'in', [False])]) + print ">>> 10:", res_10 + print ">>> 11:", res_11 + print ">>> 12:", res_12 + print ">>> 13:", res_13 + print ">>> 14:", res_14 - Property of the query (one2many not in False). - @@ -273,6 +324,8 @@ res_8 = self.search(cr, uid, [('currency_ids', 'like', default_company.currency_ids[0].name)]) res_9 = self.search(cr, uid, [('currency_ids', 'like', 'probably_unexisting_name')]) # self.search(cr, uid, [('currency_ids', 'unexisting_op', 'probably_unexisting_name')]) # TODO expected exception + + # get the companies referenced by some currency (this is normally the main company) res_10 = self.search(cr, uid, [('currency_ids', 'not like', 'probably_unexisting_name')]) res_11 = self.search(cr, uid, [('currency_ids', 'not in', [max_currency_id + 1])]) res_12 = self.search(cr, uid, [('currency_ids', 'not in', False)]) From 2334d4be738ff74e1a037c6523046cc846ee7d6f Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 9 Aug 2011 17:42:21 +0200 Subject: [PATCH 12/24] [REF] expression: cleaning. bzr revid: vmt@openerp.com-20110809154221-550a57hblf4v2foe --- openerp/osv/expression.py | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index a75c826b3d8..4c0320e3a6b 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -29,6 +29,9 @@ NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' +OPS = ('=', '!=', '<>', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') +NEGATIVE_OPS = ('!=', '<>', 'not like', 'not ilike', 'not in') + TRUE_LEAF = (1, '=', 1) FALSE_LEAF = (0, '=', 1) @@ -101,7 +104,6 @@ def is_operator(element): # TODO change the share wizard to use this function. def is_leaf(element, internal=False): - OPS = ('=', '!=', '<>', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') INTERNAL_OPS = OPS + ('inselect',) return (isinstance(element, tuple) or isinstance(element, list)) \ and len(element) == 3 \ @@ -297,15 +299,11 @@ class expression(object): self.__exp[i] = FALSE_LEAF else: call_null = False - o2m_op = 'in' - if operator in ['not like','not ilike','not in','<>','!=']: - o2m_op = 'not in' + o2m_op = 'not in' if operator in NEGATIVE_OPS else 'in' self.__exp[i] = ('id', o2m_op, select_from_where(cr, field._fields_id, field_obj._table, 'id', ids2, operator)) if call_null: - o2m_op = 'not in' - if operator in ['not like','not ilike','not in','<>','!=']: - o2m_op = 'in' + o2m_op = 'in' if operator in NEGATIVE_OPS else 'not in' self.__exp[i] = ('id', o2m_op, select_distinct_from_where_not_null(cr, field._fields_id, field_obj._table) or [0]) elif field._type == 'many2many': @@ -341,15 +339,11 @@ class expression(object): operator = 'in' # operator changed because ids are directly related to main object else: call_null_m2m = False - m2m_op = 'in' - if operator in ['not like','not ilike','not in','<>','!=']: - m2m_op = 'not in' + m2m_op = 'not in' if operator in NEGATIVE_OPS else 'in' self.__exp[i] = ('id', m2m_op, select_from_where(cr, field._id1, field._rel, field._id2, res_ids, operator) or [0]) if call_null_m2m: - m2m_op = 'not in' - if operator in ['not like','not ilike','not in','<>','!=']: - m2m_op = 'in' + m2m_op = 'in' if operator in NEGATIVE_OPS else 'not in' self.__exp[i] = ('id', m2m_op, select_distinct_from_where_not_null(cr, field._id1, field._rel) or [0]) elif field._type == 'many2one': @@ -393,22 +387,19 @@ class expression(object): if not isinstance(ele, basestring): m2o_str = False break + if m2o_str: + self.__exp[i] = _get_expression(field_obj, cr, uid, left, right, operator, context=context) elif right == []: - m2o_str = False if operator in ('not in', '!=', '<>'): # (many2one not in []) should return all records self.__exp[i] = TRUE_LEAF else: self.__exp[i] = FALSE_LEAF else: - new_op = '=' - if operator in ['not like','not ilike','not in','<>','!=']: - new_op = '!=' + new_op = '!=' if operator in NEGATIVE_OPS else '=' #Is it ok to put 'left' and not 'id' ? self.__exp[i] = (left, new_op, False) - if m2o_str: - self.__exp[i] = _get_expression(field_obj, cr, uid, left, right, operator, context=context) else: # other field type # add the time part to datetime field when it's not there: From 187dc4c19943863e6c4d3f03b38f44cee91b7cf7 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Tue, 9 Aug 2011 19:48:14 +0200 Subject: [PATCH 13/24] [REF]: expression: simplified <> into != in a pre-step, put some warnings (which might get boring, we will see). bzr revid: vmt@openerp.com-20110809174814-4uja94uo2spzcvm5 --- openerp/osv/expression.py | 42 ++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 4c0320e3a6b..7d8628dbb3c 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -20,6 +20,8 @@ # ############################################################################## +import logging + from openerp.tools import flatten, reverse_enumerate import fields @@ -29,8 +31,8 @@ NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' -OPS = ('=', '!=', '<>', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') -NEGATIVE_OPS = ('!=', '<>', 'not like', 'not ilike', 'not in') +OPS = ('=', '!=', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') +NEGATIVE_OPS = ('!=', 'not like', 'not ilike', 'not in') TRUE_LEAF = (1, '=', 1) FALSE_LEAF = (0, '=', 1) @@ -38,6 +40,8 @@ FALSE_LEAF = (0, '=', 1) TRUE_DOMAIN = [TRUE_LEAF] FALSE_DOMAIN = [FALSE_LEAF] +_logger = logging.getLogger('expression') + def normalize(domain): """Returns a normalized version of ``domain_expr``, where all implicit '&' operators have been made explicit. One property of normalized domain expressions is that they @@ -110,6 +114,12 @@ def is_leaf(element, internal=False): and (((not internal) and element[1] in OPS) \ or (internal and element[1] in INTERNAL_OPS)) +def normalize_operator(operator): + operator = operator.lower() + if operator == '<>': + operator = '!=' + return operator + def select_from_where(cr, s, f, w, ids, op): # todo: merge into parent query as sub-query res = [] @@ -201,7 +211,7 @@ class expression(object): if is_operator(e) or e == TRUE_LEAF or e == FALSE_LEAF: continue left, operator, right = e - operator = operator.lower() + operator = normalize_operator(operator) working_table = table # The table containing the field (the name provided in the left operand) fargs = left.split('.', 1) @@ -348,11 +358,10 @@ class expression(object): elif field._type == 'many2one': if operator == 'child_of': + ids2 = child_of_right_to_ids(right, 'ilike', field_obj) if field._obj != working_table._name: - ids2 = child_of_right_to_ids(right, 'ilike', field_obj) dom = child_of_domain(left, ids2, field_obj, prefix=field._obj) else: - ids2 = child_of_right_to_ids(right, 'ilike', field_obj) dom = child_of_domain('id', ids2, working_table, parent=left) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] else: @@ -364,12 +373,12 @@ class expression(object): #Special treatment to ill-formed domains operator = ( operator in ['<','>','<=','>='] ) and 'in' or operator - dict_op = {'not in':'!=','in':'=','=':'in','!=':'not in','<>':'not in'} + dict_op = {'not in':'!=','in':'=','=':'in','!=':'not in'} if isinstance(right, tuple): right = list(right) if (not isinstance(right, list)) and operator in ['not in','in']: operator = dict_op[operator] - elif isinstance(right, list) and operator in ['<>','!=','=']: #for domain (FIELD,'=',['value1','value2']) + elif isinstance(right, list) and operator in ['!=','=']: #for domain (FIELD,'=',['value1','value2']) operator = dict_op[operator] res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c)] if not res_ids: @@ -390,14 +399,14 @@ class expression(object): if m2o_str: self.__exp[i] = _get_expression(field_obj, cr, uid, left, right, operator, context=context) elif right == []: - if operator in ('not in', '!=', '<>'): - # (many2one not in []) should return all records - self.__exp[i] = TRUE_LEAF - else: - self.__exp[i] = FALSE_LEAF - else: + if operator not in ('in', 'not in'): + _logger.warning("The domain term '%s' should use a set operator ('in' or 'not in')." % (self.__exp[i]),) + # (many2one not in []) returns all records, (many2one in []) returns no record at all. + self.__exp[i] = TRUE_LEAF if operator in NEGATIVE_OPS else FALSE_LEAF + else: # right is False + if operator not in ('=', '!='): + _logger.warning("The domain term '%s' should use the '=' or '!=' operator." % (self.__exp[i],)) new_op = '!=' if operator in NEGATIVE_OPS else '=' - #Is it ok to put 'left' and not 'id' ? self.__exp[i] = (left, new_op, False) else: @@ -452,6 +461,7 @@ class expression(object): def __leaf_to_sql(self, leaf, table): left, operator, right = leaf + operator = normalize_operator(operator) if leaf == TRUE_LEAF: query = 'TRUE' @@ -501,11 +511,11 @@ class expression(object): query = '%s.%s IS NULL ' % (table._table, left) params = [] - elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator in ['<>', '!=']): + elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator == '!='): query = '(%s.%s IS NOT NULL and %s.%s != false)' % (table._table, left, table._table, left) params = [] - elif (((right == False) and (type(right)==bool)) or right is None) and (operator in ['<>', '!=']): + elif (((right == False) and (type(right)==bool)) or right is None) and (operator == '!='): query = '%s.%s IS NOT NULL' % (table._table, left) params = [] From 64346c63a40a9f7a255ed7b158fd9b6c8401b0c7 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Wed, 10 Aug 2011 09:19:10 +0200 Subject: [PATCH 14/24] [FIX] expression: perform a check on domain after the <> operator is removed. bzr revid: vmt@openerp.com-20110810071910-ykz0ji9nz7fqteuj --- openerp/osv/expression.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 7d8628dbb3c..14edfc0fb91 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -31,6 +31,7 @@ NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' +# This doesn't contain <> as it is simpified to != by normalize_operator(). OPS = ('=', '!=', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') NEGATIVE_OPS = ('!=', 'not like', 'not ilike', 'not in') @@ -149,10 +150,6 @@ class expression(object): """ def __init__(self, cr, uid, exp, table, context): - # check if the expression is valid - for x in exp: - if not (is_operator(x) or is_leaf(x)): - raise ValueError('Bad domain expression: %r, %r is not a valid operator or a valid term.' % (exp, x)) self.__field_tables = {} # used to store the table to use for the sql generation. key = index of the leaf self.__all_tables = set() self.__joins = [] @@ -167,6 +164,11 @@ class expression(object): def parse(self, cr, uid, exp, table, context): """ transform the leafs of the expression """ + # check if the expression is valid + for x in exp: + if not (is_operator(x) or is_leaf(x)): + raise ValueError('Bad domain expression: %r, %r is not a valid operator or a valid term.' % (exp, x)) + self.__exp = exp def child_of_domain(left, right, table, parent=None, prefix=''): From 44b3499a736fac6feac4194d8ded4be593007ccc Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Wed, 10 Aug 2011 10:56:04 +0200 Subject: [PATCH 15/24] [FIX] expression: better handling of []/False with in/not in. The newly added tests were wrong before the fix. The change might break existing code if it was relying on this broken behavior. This also starts a better separation of concern between parse() and __leaf_to_sql. bzr revid: vmt@openerp.com-20110810085604-1f6ahwzuzfklj1b7 --- .../addons/base/test/test_osv_expression.yml | 19 +++++ openerp/osv/expression.py | 74 ++++++++++--------- 2 files changed, 59 insertions(+), 34 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 433b979da31..ee54d7d3d58 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -184,6 +184,7 @@ - !python {model: res.partner }: | partner_ids = self.search(cr, uid, []) + partner_ids.sort() max_partner_id = max(partner_ids) partners = self.browse(cr, uid, partner_ids) with_parent = [] @@ -194,6 +195,11 @@ for x in partners: if not x.parent_id: without_parent.append(x.id) + with_website = [] + for x in partners: + if x.website: + with_website.append(x.id) + with_website.sort() print "with_parent", with_parent print "without_parent", without_parent res_0 = self.search(cr, uid, [('parent_id', 'not like', 'probably_unexisting_name')]) @@ -228,6 +234,19 @@ print ">>> 12:", res_12 print ">>> 13:", res_13 print ">>> 14:", res_14 + + # Testing many2one field is not enough, a regular char field is tested + # with in [] and must not return any result. + res_15 = self.search(cr, uid, [('website', 'in', [])]) + assert res_15 == [] + # not in [] must return everything. + res_16 = self.search(cr, uid, [('website', 'not in', [])]) + res_16.sort() + assert res_16 == partner_ids + + res_17 = self.search(cr, uid, [('website', 'not in', False)]) + res_17.sort() + assert res_17 == with_website - Property of the query (one2many not in False). - diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 14edfc0fb91..4706117dd6b 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -109,7 +109,7 @@ def is_operator(element): # TODO change the share wizard to use this function. def is_leaf(element, internal=False): - INTERNAL_OPS = OPS + ('inselect',) + INTERNAL_OPS = OPS + ('inselect', '<>') return (isinstance(element, tuple) or isinstance(element, list)) \ and len(element) == 3 \ and (((not internal) and element[1] in OPS) \ @@ -164,11 +164,6 @@ class expression(object): def parse(self, cr, uid, exp, table, context): """ transform the leafs of the expression """ - # check if the expression is valid - for x in exp: - if not (is_operator(x) or is_leaf(x)): - raise ValueError('Bad domain expression: %r, %r is not a valid operator or a valid term.' % (exp, x)) - self.__exp = exp def child_of_domain(left, right, table, parent=None, prefix=''): @@ -212,8 +207,14 @@ class expression(object): e = self.__exp[i] if is_operator(e) or e == TRUE_LEAF or e == FALSE_LEAF: continue + + # check if the expression is valid + if not is_leaf(e): + raise ValueError('Bad domain expression: %r, %r is not a valid term.' % (exp, e)) + left, operator, right = e operator = normalize_operator(operator) + working_table = table # The table containing the field (the name provided in the left operand) fargs = left.split('.', 1) @@ -401,10 +402,7 @@ class expression(object): if m2o_str: self.__exp[i] = _get_expression(field_obj, cr, uid, left, right, operator, context=context) elif right == []: - if operator not in ('in', 'not in'): - _logger.warning("The domain term '%s' should use a set operator ('in' or 'not in')." % (self.__exp[i]),) - # (many2one not in []) returns all records, (many2one in []) returns no record at all. - self.__exp[i] = TRUE_LEAF if operator in NEGATIVE_OPS else FALSE_LEAF + pass # Handled by __leaf_to_sql(). else: # right is False if operator not in ('=', '!='): _logger.warning("The domain term '%s' should use the '=' or '!=' operator." % (self.__exp[i],)) @@ -478,32 +476,40 @@ class expression(object): params = right[1] elif operator in ['in', 'not in']: - params = right and right[:] or [] - len_before = len(params) - for i in range(len_before)[::-1]: - if params[i] == False: - del params[i] - - len_after = len(params) - check_nulls = len_after != len_before - query = 'FALSE' - - # TODO this code seems broken: 'not in [False]' will become 'true', - # i.e. 'not null or null', while I expect it to be 'not null'. - if len_after: - if left == 'id': - instr = ','.join(['%s'] * len_after) - else: - instr = ','.join([table._columns[left]._symbol_set[0]] * len_after) - query = '(%s.%s %s (%s))' % (table._table, left, operator, instr) - else: - # the case for [field, 'in', []] or [left, 'not in', []] + # Two cases: right is a boolean or a list. The boolean case is an + # abuse and handled for backward compatibility. + if isinstance(right, bool): + _logger.warning("The domain term '%s' should use the '=' or '!=' operator." % (leaf,)) if operator == 'in': - query = '(%s.%s IS NULL)' % (table._table, left) + r = 'NOT NULL' if right else 'NULL' else: - query = '(%s.%s IS NOT NULL)' % (table._table, left) - if check_nulls: - query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) + r = 'NULL' if right else 'NOT NULL' + query = '(%s.%s IS %s)' % (table._table, left, r) + params = [] + elif isinstance(right, (list, tuple)): + params = right[:] + check_nulls = False + for i in range(len(params))[::-1]: + if params[i] == False: + check_nulls = True + del params[i] + + if params: + if left == 'id': + instr = ','.join(['%s'] * len(params)) + else: + instr = ','.join([table._columns[left]._symbol_set[0]] * len(params)) + query = '(%s.%s %s (%s))' % (table._table, left, operator, instr) + else: + # The case for (left, 'in', []) or (left, 'not in', []). + if operator == 'in': + query = 'FALSE' + else: + query = 'TRUE' + if check_nulls: + query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) # TODO not necessary for TRUE + else: # Must not happen. + pass elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator == '='): query = '(%s.%s IS NULL or %s.%s = false )' % (table._table, left, table._table, left) From a32476c6d99231536b26a3f20a48914a695925ad Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Wed, 10 Aug 2011 11:28:59 +0200 Subject: [PATCH 16/24] [FIX] expression: the <> can be part of leaves (before normalization). bzr revid: vmt@openerp.com-20110810092859-mzp9nmo81azxtq8s --- openerp/osv/expression.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 4706117dd6b..14c358a2c42 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -109,11 +109,11 @@ def is_operator(element): # TODO change the share wizard to use this function. def is_leaf(element, internal=False): - INTERNAL_OPS = OPS + ('inselect', '<>') + INTERNAL_OPS = OPS + ('inselect',) return (isinstance(element, tuple) or isinstance(element, list)) \ and len(element) == 3 \ - and (((not internal) and element[1] in OPS) \ - or (internal and element[1] in INTERNAL_OPS)) + and (((not internal) and element[1] in OPS + ('<>',)) \ + or (internal and element[1] in INTERNAL_OPS + ('<>',))) def normalize_operator(operator): operator = operator.lower() @@ -515,7 +515,7 @@ class expression(object): query = '(%s.%s IS NULL or %s.%s = false )' % (table._table, left, table._table, left) params = [] - elif (((right == False) and (type(right)==bool)) or (right is None)) and (operator == '='): + elif (right is False or right is None) and (operator == '='): query = '%s.%s IS NULL ' % (table._table, left) params = [] @@ -523,7 +523,7 @@ class expression(object): query = '(%s.%s IS NOT NULL and %s.%s != false)' % (table._table, left, table._table, left) params = [] - elif (((right == False) and (type(right)==bool)) or right is None) and (operator == '!='): + elif (right is False or right is None) and (operator == '!='): query = '%s.%s IS NOT NULL' % (table._table, left) params = [] From c7ccae6feb5e8072ccd9b5b3d2cf499f828c425d Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Wed, 10 Aug 2011 12:29:22 +0200 Subject: [PATCH 17/24] [REF] expression: unnecessary code in parse() because __leaf_to_sql() is already doing the work. bzr revid: vmt@openerp.com-20110810102922-xe2oxkgorarco4vm --- openerp/osv/expression.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 14c358a2c42..8a5aa621963 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -384,10 +384,7 @@ class expression(object): elif isinstance(right, list) and operator in ['!=','=']: #for domain (FIELD,'=',['value1','value2']) operator = dict_op[operator] res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c)] - if not res_ids: - return FALSE_LEAF - else: - return (left, 'in', res_ids) + return (left, 'in', res_ids) m2o_str = False if right: @@ -404,10 +401,7 @@ class expression(object): elif right == []: pass # Handled by __leaf_to_sql(). else: # right is False - if operator not in ('=', '!='): - _logger.warning("The domain term '%s' should use the '=' or '!=' operator." % (self.__exp[i],)) - new_op = '!=' if operator in NEGATIVE_OPS else '=' - self.__exp[i] = (left, new_op, False) + pass # Handled by __leaf_to_sql(). else: # other field type @@ -502,10 +496,7 @@ class expression(object): query = '(%s.%s %s (%s))' % (table._table, left, operator, instr) else: # The case for (left, 'in', []) or (left, 'not in', []). - if operator == 'in': - query = 'FALSE' - else: - query = 'TRUE' + query = 'FALSE' if operator == 'in' else 'TRUE' if check_nulls: query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) # TODO not necessary for TRUE else: # Must not happen. From 784a2d6add9f36eda80e2197210e794bfc530900 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Wed, 10 Aug 2011 14:00:57 +0200 Subject: [PATCH 18/24] [REF] expression: normalize the operator w.r.t. the right operand so that it is done in one place. bzr revid: vmt@openerp.com-20110810120057-ruijgl9w4zce1teg --- .../addons/base/test/test_osv_expression.yml | 2 ++ openerp/osv/expression.py | 30 ++++++++++--------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index ee54d7d3d58..9077c843f72 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -365,7 +365,9 @@ print ">>> 11:", res_11 #assert res_10 == res_11 assert res_10 == res_12 + print ">>> 12:", res_12 assert res_10 == res_13 + print ">>> 13:", res_13 # child_of x returns x and its children (direct or not). company = self.browse(cr, uid, ref('ymltest_company3')) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 8a5aa621963..aed4b28fa89 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -115,11 +115,18 @@ def is_leaf(element, internal=False): and (((not internal) and element[1] in OPS + ('<>',)) \ or (internal and element[1] in INTERNAL_OPS + ('<>',))) -def normalize_operator(operator): +def normalize_leaf(left, operator, right): + original = operator operator = operator.lower() if operator == '<>': operator = '!=' - return operator + if isinstance(right, bool) and operator in ('in', 'not in'): + _logger.warning("The domain term '%s' should use the '=' or '!=' operator." % ((left, original, right),)) + operator = '=' if operator == 'in' else '!=' + if isinstance(right, (list, tuple)) and operator in ('=', '!='): + _logger.warning("The domain term '%s' should use the 'in' or 'not in' operator." % ((left, original, right),)) + operator = 'in' if operator == '=' else 'not in' + return left, operator, right def select_from_where(cr, s, f, w, ids, op): # todo: merge into parent query as sub-query @@ -212,8 +219,10 @@ class expression(object): if not is_leaf(e): raise ValueError('Bad domain expression: %r, %r is not a valid term.' % (exp, e)) + # normalize the leaf's operator + e = normalize_leaf(*e) + self.__exp[i] = e left, operator, right = e - operator = normalize_operator(operator) working_table = table # The table containing the field (the name provided in the left operand) fargs = left.split('.', 1) @@ -246,18 +255,12 @@ class expression(object): if len(fargs) > 1: if field._type == 'many2one': right = field_obj.search(cr, uid, [(fargs[1], operator, right)], context=context) - if right == []: - self.__exp[i] = FALSE_LEAF - else: - self.__exp[i] = (fargs[0], 'in', right) + self.__exp[i] = (fargs[0], 'in', right) # Making search easier when there is a left operand as field.o2m or field.m2m if field._type in ['many2many', 'one2many']: right = field_obj.search(cr, uid, [(fargs[1], operator, right)], context=context) right1 = table.search(cr, uid, [(fargs[0], 'in', right)], context=context) - if right1 == []: - self.__exp[i] = FALSE_LEAF - else: - self.__exp[i] = ('id', 'in', right1) + self.__exp[i] = ('id', 'in', right1) if not isinstance(field, fields.property): continue @@ -317,7 +320,7 @@ class expression(object): if call_null: o2m_op = 'in' if operator in NEGATIVE_OPS else 'not in' - self.__exp[i] = ('id', o2m_op, select_distinct_from_where_not_null(cr, field._fields_id, field_obj._table) or [0]) + self.__exp[i] = ('id', o2m_op, select_distinct_from_where_not_null(cr, field._fields_id, field_obj._table)) elif field._type == 'many2many': #FIXME @@ -357,7 +360,7 @@ class expression(object): if call_null_m2m: m2m_op = 'in' if operator in NEGATIVE_OPS else 'not in' - self.__exp[i] = ('id', m2m_op, select_distinct_from_where_not_null(cr, field._id1, field._rel) or [0]) + self.__exp[i] = ('id', m2m_op, select_distinct_from_where_not_null(cr, field._id1, field._rel)) elif field._type == 'many2one': if operator == 'child_of': @@ -455,7 +458,6 @@ class expression(object): def __leaf_to_sql(self, leaf, table): left, operator, right = leaf - operator = normalize_operator(operator) if leaf == TRUE_LEAF: query = 'TRUE' From 19a21de5e13bb9aee0a23bc19897375acea93444 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Wed, 10 Aug 2011 15:06:46 +0200 Subject: [PATCH 19/24] [FIX] expression: better semantic for in/not in w.r.t. null values: In SQL, not in (2, 7, 9) will return only non-null records. In OpenERP, a domain with not in [2, 7, 9] should return every record whose value is not 2 or 7 or 9, including the one with a null value. This means that the union of (not in XXX) and (in XXX) will return the whole set. bzr revid: vmt@openerp.com-20110810130646-2y822a276igz2f1g --- .../addons/base/test/test_osv_expression.yml | 52 +++++++++++++++---- openerp/osv/expression.py | 11 +++- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 9077c843f72..2aab8293fb4 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -192,9 +192,11 @@ for x in partners: if x.parent_id and x.parent_id.id in partner_ids: with_parent.append(x.id) + with_parent.sort() for x in partners: if not x.parent_id: without_parent.append(x.id) + without_parent.sort() with_website = [] for x in partners: if x.website: @@ -202,17 +204,45 @@ with_website.sort() print "with_parent", with_parent print "without_parent", without_parent - res_0 = self.search(cr, uid, [('parent_id', 'not like', 'probably_unexisting_name')]) - res_1 = self.search(cr, uid, [('parent_id', 'not in', [max_partner_id + 1])]) - res_2 = self.search(cr, uid, [('parent_id', 'not in', False)]) - res_3 = self.search(cr, uid, [('parent_id', 'not in', [])]) - res_4 = self.search(cr, uid, [('parent_id', 'not in', [False])]) - print ">>> 0:", res_0 - print ">>> 1:", res_1 - print ">>> 2:", res_2 - print ">>> 3:", res_3 - print ">>> 4:", res_4 + + # We treat null values differently than in SQL. For instance in SQL: + # SELECT id FROM res_partner WHERE parent_id NOT IN (0) + # will return only the records with non-null parent_id. + # SELECT id FROM res_partner WHERE parent_id IN (0) + # will return expectedly nothing (our ids always begin at 1). + # This means the union of those two results will give only some + # records, but not all present in database. + # + # When using domains and the ORM's search method, we think it is + # more intuitive that the union returns all the records, and that + # a domain like ('parent_id', 'not in', [0]) will return all + # the records. For instance, if you perform a search for the companies + # that don't have OpenERP has a parent company, you expect to find, + # among others, the companies that don't have parent company. + # + # ('parent_id', 'not in', [0]) must give the same result than + # ('parent_id', 'not in', []), i.e. a empty set or a set with non- + # existing values be treated similarly if we simply check that some + # existing value belongs to them. + + res_0 = self.search(cr, uid, [('parent_id', 'not like', 'probably_unexisting_name')]) # get all rows, included null parent_id + res_0.sort() + res_1 = self.search(cr, uid, [('parent_id', 'not in', [max_partner_id + 1])]) # get all rows, included null parent_id + res_1.sort() + res_2 = self.search(cr, uid, [('parent_id', 'not in', False)]) # get rows with not null parent_id, deprecated syntax + res_2.sort() + res_3 = self.search(cr, uid, [('parent_id', 'not in', [])]) # get all rows, included null parent_id + res_3.sort() + res_4 = self.search(cr, uid, [('parent_id', 'not in', [False])]) # get rows with not null parent_id + res_4.sort() + assert res_0 == partner_ids + assert res_1 == partner_ids + assert res_2 == with_parent + assert res_3 == partner_ids + assert res_4 == with_parent print ">>> ----------" + # The results of these queries, when combined with queries 0..4 must + # give the whole set of ids. res_5 = self.search(cr, uid, [('parent_id', 'like', 'probably_unexisting_name')]) res_6 = self.search(cr, uid, [('parent_id', 'in', [max_partner_id + 1])]) res_7 = self.search(cr, uid, [('parent_id', 'in', False)]) @@ -224,6 +254,8 @@ print ">>> 8:", res_8 print ">>> 9:", res_9 print ">>> ----------" + # These queries must return exactly the results than the queries 0..4, + # i.e. not ... in ... must be the same as ... not in ... . res_10 = self.search(cr, uid, ['!', ('parent_id', 'like', 'probably_unexisting_name')]) res_11 = self.search(cr, uid, ['!', ('parent_id', 'in', [max_partner_id + 1])]) res_12 = self.search(cr, uid, ['!', ('parent_id', 'in', False)]) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index aed4b28fa89..8be06d5741c 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -387,6 +387,8 @@ class expression(object): elif isinstance(right, list) and operator in ['!=','=']: #for domain (FIELD,'=',['value1','value2']) operator = dict_op[operator] res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c)] + if operator in NEGATIVE_OPS: + res_ids.append(False) # TODO this should not be appended if False was in 'right' return (left, 'in', res_ids) m2o_str = False @@ -499,8 +501,13 @@ class expression(object): else: # The case for (left, 'in', []) or (left, 'not in', []). query = 'FALSE' if operator == 'in' else 'TRUE' - if check_nulls: - query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) # TODO not necessary for TRUE + + if check_nulls and operator == 'in': + query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) + elif not check_nulls and operator == 'not in': + query = '(%s OR %s.%s IS NULL)' % (query, table._table, left) + elif check_nulls and operator == 'not in': + query = '(%s AND %s.%s IS NOT NULL)' % (query, table._table, left) # needed only for TRUE. else: # Must not happen. pass From 5de1e1a3676a4a13601a81e12993f3276a2ebca4 Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Wed, 10 Aug 2011 16:38:38 +0200 Subject: [PATCH 20/24] [IMP] expression: some more tests pass with the correct semantic. bzr revid: vmt@openerp.com-20110810143838-l5ujglk5w85mchv1 --- .../addons/base/test/test_osv_expression.yml | 32 +++++++++----- openerp/osv/expression.py | 44 ++++++++++++++++++- 2 files changed, 63 insertions(+), 13 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 2aab8293fb4..6ff02fb45f6 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -240,32 +240,40 @@ assert res_2 == with_parent assert res_3 == partner_ids assert res_4 == with_parent - print ">>> ----------" # The results of these queries, when combined with queries 0..4 must # give the whole set of ids. res_5 = self.search(cr, uid, [('parent_id', 'like', 'probably_unexisting_name')]) + res_5.sort() res_6 = self.search(cr, uid, [('parent_id', 'in', [max_partner_id + 1])]) + res_6.sort() res_7 = self.search(cr, uid, [('parent_id', 'in', False)]) + res_7.sort() res_8 = self.search(cr, uid, [('parent_id', 'in', [])]) + res_8.sort() res_9 = self.search(cr, uid, [('parent_id', 'in', [False])]) - print ">>> 5:", res_5 - print ">>> 6:", res_6 - print ">>> 7:", res_7 - print ">>> 8:", res_8 - print ">>> 9:", res_9 - print ">>> ----------" + res_9.sort() + assert res_5 == [] + assert res_6 == [] + assert res_7 == without_parent + assert res_8 == [] + assert res_9 == without_parent # These queries must return exactly the results than the queries 0..4, # i.e. not ... in ... must be the same as ... not in ... . res_10 = self.search(cr, uid, ['!', ('parent_id', 'like', 'probably_unexisting_name')]) + res_10.sort() res_11 = self.search(cr, uid, ['!', ('parent_id', 'in', [max_partner_id + 1])]) + res_11.sort() res_12 = self.search(cr, uid, ['!', ('parent_id', 'in', False)]) + res_12.sort() res_13 = self.search(cr, uid, ['!', ('parent_id', 'in', [])]) + res_13.sort() res_14 = self.search(cr, uid, ['!', ('parent_id', 'in', [False])]) - print ">>> 10:", res_10 - print ">>> 11:", res_11 - print ">>> 12:", res_12 - print ">>> 13:", res_13 - print ">>> 14:", res_14 + res_14.sort() + assert res_0 == res_10 + assert res_1 == res_11 + assert res_2 == res_12 + assert res_3 == res_13 + assert res_4 == res_14 # Testing many2one field is not enough, a regular char field is tested # with in [] and must not return any result. diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 8be06d5741c..0b613797f7b 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -128,6 +128,48 @@ def normalize_leaf(left, operator, right): operator = 'in' if operator == '=' else 'not in' return left, operator, right +def distribute_not(domain): + """ Distribute the '!' operator on a normalized domain. + """ + def negate(leaf): + left, operator, right = leaf + mapping = { + '<': '>=', + '>': '<=', + '<=': '>', + '>=': '<', + '=': '!=', + '!=': '=', + } + if operator in ('in', 'like', 'ilike'): + operator = 'not ' + operator + return [(left, operator, right)] + if operator in ('not in', 'not like', 'not ilike'): + operator = operator[4:] + return [(left, operator, right)] + if operator in mapping: + operator = mapping[operator] + return [(left, operator, right)] + return ['!', (left, operator, right)] + def distribute(domain): + if is_leaf(domain[0]): + return negate(domain[0]), domain[1:] + if domain[0] == '&': + done1, todo1 = distribute(domain[1:]) + done2, todo2 = distribute(todo1) + return ['|'] + done1 + done2, todo2 + if domain[0] == '|': + done1, todo1 = distribute(domain[1:]) + done2, todo2 = distribute(todo1) + return ['&'] + done1 + done2, todo2 + if not domain: + return [] + if domain[0] != '!': + return [domain[0]] + distribute_not(domain[1:]) + if domain[0] == '!': + done, todo = distribute(domain[1:]) + return done + distribute_not(todo) + def select_from_where(cr, s, f, w, ids, op): # todo: merge into parent query as sub-query res = [] @@ -162,7 +204,7 @@ class expression(object): self.__joins = [] self.__main_table = None # 'root' table. set by parse() # assign self.__exp with the normalized, parsed domain. - self.parse(cr, uid, normalize(exp), table, context) + self.parse(cr, uid, distribute_not(normalize(exp)), table, context) # TODO used only for osv_memory @property From d462db158ec4ad3d91a047a822d34004c9a0deae Mon Sep 17 00:00:00 2001 From: Vo Minh Thu Date: Thu, 11 Aug 2011 11:31:36 +0200 Subject: [PATCH 21/24] [REF] expression: comments, minor changes. bzr revid: vmt@openerp.com-20110811093136-8whjybyvf3kf3t4y --- openerp/osv/expression.py | 132 +++++++++++++++++++++++++++++++------- 1 file changed, 109 insertions(+), 23 deletions(-) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 0b613797f7b..ca1360288ff 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -20,10 +20,65 @@ # ############################################################################## +""" Domain expression processing + +The main duty of this module is to compile a domain expression into a SQL +query. A lot of things should be documented here, but as a first step in the +right direction, some tests in test_osv_expression.yml might give you some +additional information. + +For legacy reasons, a domain uses an inconsistent two-levels abstract syntax +(domains are regular Python data structures). At the first level, a domain +is an expression made of terms (sometimes called leaves) and (domain) operators +used in prefix notation. The available operators at this level are '!', '&', +and '|'. '!' is a unary 'not', '&' is a binary 'and', and '|' is a binary 'or'. +For instance, here is a possible domain. ( stands for an arbitrary term, +more on this later.) + + ['&', '!', , '|', , ] + +It is equivalent to this pseudo code using infix notation: + + (not ) and ( or ) + +The second level of syntax deals with the term representation. A term is +a triple of the form (left, operator, right). That is, a term uses an infix +notation, and the available operators, and possible left and right operands +differ with those of the previous level. Here is a possible term: + + ('company_id.name', '=', 'OpenERP') + +The left and right operand don't have the same possible values. The left +operand is field name (related to the model for which the domain applies). +Actually, the field name can use the dot-notation to traverse relationships. +The right operand is a Python value whose type should match the used operator +and field type. In the above example, a string is used because the name field +of a company has type string, and because we use the '=' operator. When +appropriate, a 'in' operator can be used, and thus the right operand should be +a list. + +Note: the non-uniform syntax could have been more uniform, but this would hide +an important limitation of the domain syntax. Say that the term representation +was ['=', 'company_id.name', 'OpenERP']. Used in a complete domain, this would +look like: + + ['!', ['=', 'company_id.name', 'OpenERP']] + +and you would be tempted to believe something like this would be possible: + + ['!', ['=', 'company_id.name', ['&', ..., ...]]] + +That is, a domain could be a valid operand. But this is not the case. A domain +is really limited to a two-level nature, and can not takes a recursive form: a +domain is not a valid second-level operand. + +""" + import logging from openerp.tools import flatten, reverse_enumerate import fields +import openerp.modules #.apidoc title: Domain Expressions @@ -31,8 +86,19 @@ NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' -# This doesn't contain <> as it is simpified to != by normalize_operator(). +# List of available term operators. It is also possible to use the '<>' +# operator, which is strictly the same as '!='; the later should be prefered +# for consistency. This list doesn't contain '<>' as it is simpified to '!=' +# by the normalize_operator() function (so later part of the code deals with +# only one representation). +# An internal (i.e. not available to the user) 'inselect' operator is also +# used. In this case its right operand has the form (subselect, params). OPS = ('=', '!=', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') + +# A subset of the above operators, with a 'negative' semantic. When the +# expressions 'in NEGATIVE_OPS' or 'not in NEGATIVE_OPS' are used in the code +# below, this doesn't necessarily mean that any of those NEGATIVE_OPS is +# legal in the processed term. NEGATIVE_OPS = ('!=', 'not like', 'not ilike', 'not in') TRUE_LEAF = (1, '=', 1) @@ -105,10 +171,17 @@ def OR(domains): return combine(OR_OPERATOR, FALSE_DOMAIN, TRUE_DOMAIN, domains) def is_operator(element): + """ Test whether an object is a valid domain operator. """ return isinstance(element, (str, unicode)) and element in [AND_OPERATOR, OR_OPERATOR, NOT_OPERATOR] # TODO change the share wizard to use this function. def is_leaf(element, internal=False): + """ Test whether an object is a valid domain term. + + :param internal: allow or not the 'inselect' internal operator in the term. + This normally should be always left to False. + + """ INTERNAL_OPS = OPS + ('inselect',) return (isinstance(element, tuple) or isinstance(element, list)) \ and len(element) == 3 \ @@ -116,6 +189,9 @@ def is_leaf(element, internal=False): or (internal and element[1] in INTERNAL_OPS + ('<>',))) def normalize_leaf(left, operator, right): + """ Change a term's operator to some canonical form, simplifying later + processing. + """ original = operator operator = operator.lower() if operator == '<>': @@ -130,6 +206,17 @@ def normalize_leaf(left, operator, right): def distribute_not(domain): """ Distribute the '!' operator on a normalized domain. + + Because we don't use SQL semantic for processing a 'left not in right' + query (i.e. our 'not in' is not simply translated to a SQL 'not in'), + it means that a '! left in right' can not be simply processed + by __leaf_to_sql by first emitting code for 'left in right' then wrapping + the result with 'not (...)', as it would result in a 'not in' at the SQL + level. + + This function is thus responsible for pushing the '!' operator inside the + terms. + """ def negate(leaf): left, operator, right = leaf @@ -199,6 +286,7 @@ class expression(object): """ def __init__(self, cr, uid, exp, table, context): + self.has_unaccent = openerp.modules.registry.RegistryManager.get(cr.dbname).has_unaccent self.__field_tables = {} # used to store the table to use for the sql generation. key = index of the leaf self.__all_tables = set() self.__joins = [] @@ -214,6 +302,8 @@ class expression(object): def parse(self, cr, uid, exp, table, context): """ transform the leafs of the expression """ self.__exp = exp + self.__main_table = table + self.__all_tables.add(table) def child_of_domain(left, right, table, parent=None, prefix=''): ids = right @@ -237,19 +327,16 @@ class expression(object): return [(left, 'in', rg(ids, table, parent or table._parent_name))] # TODO rename this function as it is not strictly for 'child_of', but also for 'in'... - def child_of_right_to_ids(value, operator, field_obj): + def child_of_right_to_ids(value, field_obj): """ Normalize a single id, or a string, or a list of ids to a list of ids. """ if isinstance(value, basestring): - return [x[0] for x in field_obj.name_search(cr, uid, value, [], operator, context=context, limit=None)] + return [x[0] for x in field_obj.name_search(cr, uid, value, [], 'ilike', context=context, limit=None)] elif isinstance(value, (int, long)): return [value] else: return list(value) - self.__main_table = table - self.__all_tables.add(table) - i = -1 while i + 1 Date: Thu, 11 Aug 2011 13:01:18 +0200 Subject: [PATCH 22/24] [IMP] unaccent: support for the postgres unaccent contrib module. OpenERP is now able to use the SQL unaccent() function when available, and when the server is run with the --unaccent flag. bzr revid: vmt@openerp.com-20110811110118-cyx2l6c3wk58083p --- .../addons/base/test/test_osv_expression.yml | 19 ++++++- openerp/modules/registry.py | 10 +++- openerp/osv/expression.py | 52 ++++++++++++++++++- openerp/tools/config.py | 4 +- 4 files changed, 78 insertions(+), 7 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 6ff02fb45f6..68d78df9baf 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -435,6 +435,23 @@ domain = [('x','in',['y','z']),('a.v','=','e'),'|','|',('a','=','b'),'!',('c','>','d'),('e','!=','f'),('g','=','h')] norm_domain = ['&','&','&'] + domain assert norm_domain == expression.normalize(domain), "Non-normalized domains should be properly normalized" - +- + Unaccent. Create a company with an accent in its name. +- + !record {model: res.company, id: ymltest_unaccent_company}: + name: Hélène +- + Test the unaccent-enabled 'ilike'. +- + !python {model: res.company}: | + if self.pool.has_unaccent: + ids = self.search(cr, uid, [('name','ilike','Helene')], {}) + assert ids == [ref('ymltest_unaccent_company')] + ids = self.search(cr, uid, [('name','ilike','hélène')], {}) + assert ids == [ref('ymltest_unaccent_company')] + ids = self.search(cr, uid, [('name','not ilike','Helene')], {}) + assert ref('ymltest_unaccent_company') not in ids + ids = self.search(cr, uid, [('name','not ilike','hélène')], {}) + assert ref('ymltest_unaccent_company') not in ids diff --git a/openerp/modules/registry.py b/openerp/modules/registry.py index bc80a7bc354..735bf68cfdc 100644 --- a/openerp/modules/registry.py +++ b/openerp/modules/registry.py @@ -23,10 +23,12 @@ """ +import logging + import openerp.sql_db import openerp.osv.orm import openerp.modules.db - +import openerp.tools.config class Registry(object): """ Model registry for a particular database. @@ -46,7 +48,11 @@ class Registry(object): self.db = openerp.sql_db.db_connect(db_name) cr = self.db.cursor() - self.has_unaccent = openerp.modules.db.has_unaccent(cr) + has_unaccent = openerp.modules.db.has_unaccent(cr) + if openerp.tools.config['unaccent'] and not has_unaccent: + logger = logging.getLogger('unaccent') + logger.warning("The option --unaccent was given but no unaccent() function was found in database.") + self.has_unaccent = openerp.tools.config['unaccent'] and has_unaccent cr.close() def do_parent_store(self, cr): diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index ca1360288ff..983dbb36181 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -72,6 +72,47 @@ That is, a domain could be a valid operand. But this is not the case. A domain is really limited to a two-level nature, and can not takes a recursive form: a domain is not a valid second-level operand. +Unaccent - Accent-insensitive search + +OpenERP will use the SQL function 'unaccent' when available for the 'ilike' and +'not ilike' operators. Normally the 'unaccent' function is obtained from the +PostgreSQL 'unaccent' contrib module[0]. The steps to install the module might +differ on specific PostgreSQL versions. We give here some instruction for +PostgreSQL 9.x on a Ubuntu system. + +Ubuntu doesn't come yet with PostgreSQL 9.x, so an alternive package source +is used. We use Martin Pitt's PPA available at ppa:pitti/postgresql[1]. See +[2] for instructions. Basically: + + > sudo add-apt-repository ppa:pitti/postgresql + > sudo apt-get update + +Once the package list is up-to-date, you have to install PostgreSQL 9.0 and +its contrib modules. + + > sudo apt-get install postgresql-9.0 postgresql-contrib-9.0 + +When you want to enable unaccent on some database: + + > psql9 -f /usr/share/postgresql/9.0/contrib/unaccent.sql + +Here 'psql9' is an alias for the newly installed PostgreSQL 9.0 tool, together +with the correct port if necessary (for instance if PostgreSQL 8.4 is running +on 5432). (Other aliases can be used for createdb and dropdb.) + + > alias psql9='/usr/lib/postgresql/9.0/bin/psql -p 5433' + +You can check unaccent is working: + + > psql9 -c"select unaccent('hélène')" + +Finally, to instruct OpenERP to really use the unaccent function, you have to +start the server specifying the --unaccent flag. + +[0] http://developer.postgresql.org/pgdocs/postgres/unaccent.html +[1] https://launchpad.net/~pitti/+archive/postgresql +[2] https://launchpad.net/+help/soyuz/ppa-sources-list.html + """ import logging @@ -82,6 +123,7 @@ import openerp.modules #.apidoc title: Domain Expressions +# Domain operators. NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' @@ -678,9 +720,15 @@ class expression(object): op = {'=like':'like','=ilike':'ilike'}.get(operator, operator) if left in table._columns: format = like and '%s' or table._columns[left]._symbol_set[0] - query = '(%s.%s %s %s)' % (table._table, left, op, format) + if self.has_unaccent and op in ('ilike', 'not ilike'): + query = '(unaccent(%s.%s) %s unaccent(%s))' % (table._table, left, op, format) + else: + query = '(%s.%s %s %s)' % (table._table, left, op, format) else: - query = "(%s.%s %s '%s')" % (table._table, left, op, right) + if self.has_unaccent and op in ('ilike', 'not ilike'): + query = "(unaccent(%s.%s) %s unaccent('%s'))" % (table._table, left, op, right) + else: + query = "(%s.%s %s '%s')" % (table._table, left, op, right) add_null = False if like: diff --git a/openerp/tools/config.py b/openerp/tools/config.py index b09ef040cea..f7335bcf0c5 100644 --- a/openerp/tools/config.py +++ b/openerp/tools/config.py @@ -250,7 +250,7 @@ class configmanager(object): "osv_memory tables. This is a decimal value expressed in hours, " "and the default is 1 hour.", type="float") - group.add_option("--unaccent", dest="unaccent", my_default=False, + group.add_option("--unaccent", dest="unaccent", my_default=False, action="store_true", help="Use the unaccent function provided by the database when available.") parser.add_option_group(group) @@ -337,7 +337,7 @@ class configmanager(object): 'stop_after_init', 'logrotate', 'without_demo', 'netrpc', 'xmlrpc', 'syslog', 'list_db', 'xmlrpcs', 'test_file', 'test_disable', 'test_commit', 'test_report_directory', - 'osv_memory_count_limit', 'osv_memory_age_limit', + 'osv_memory_count_limit', 'osv_memory_age_limit', 'unaccent', ] for arg in keys: From 6652943cd24d02fa7286d95d30fb3823d6300214 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Thu, 22 Sep 2011 00:44:23 +0200 Subject: [PATCH 23/24] [IMP] minor cosmetic changes to improve readability and ease reviewing process bzr revid: odo@openerp.com-20110921224423-h5hjnqrqgrp5hhau --- .../addons/base/test/test_osv_expression.yml | 37 +++-- openerp/osv/expression.py | 152 ++++++++++-------- 2 files changed, 104 insertions(+), 85 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index ee13f87e252..ac2542d54e0 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -202,8 +202,6 @@ if x.website: with_website.append(x.id) with_website.sort() - print "with_parent", with_parent - print "without_parent", without_parent # We treat null values differently than in SQL. For instance in SQL: # SELECT id FROM res_partner WHERE parent_id NOT IN (0) @@ -367,22 +365,31 @@ # Search the company via its one2many (the one2many must point back at the company). company = self.browse(cr, uid, ref('ymltest_company3')) max_currency_id = max(self.pool.get('res.currency').search(cr, uid, [])) - currency_ids = self.pool.get('res.currency').search(cr, uid, [('name', 'not like', 'probably_unexisting_name')]) - currency_ids = self.pool.get('res.currency').search(cr, uid, [('id', 'not in', [max_currency_id + 1003])]) - currency_ids = self.pool.get('res.currency').search(cr, uid, [('id', 'not in', [])]) + currency_ids1 = self.pool.get('res.currency').search(cr, uid, [('name', 'not like', 'probably_unexisting_name')]) + currency_ids2 = self.pool.get('res.currency').search(cr, uid, [('id', 'not in', [max_currency_id + 1003])]) + currency_ids3 = self.pool.get('res.currency').search(cr, uid, [('id', 'not in', [])]) + assert currency_ids1 == currency_ids2 == currency_ids3, 'All 3 results should have be the same: all currencies' default_company = self.browse(cr, uid, 1) # one2many towards same model - res_1 = self.search(cr, uid, [('child_ids', 'in', [x.id for x in company.child_ids])]) - res_2 = self.search(cr, uid, [('child_ids', 'in', [company.child_ids[0].id])]) + res_1 = self.search(cr, uid, [('child_ids', 'in', [x.id for x in company.child_ids])]) # any company having a child of company3 as child + res_2 = self.search(cr, uid, [('child_ids', 'in', [company.child_ids[0].id])]) # any company having the first child of company3 as child # one2many towards another model - res_3 = self.search(cr, uid, [('currency_ids', 'in', [x.id for x in default_company.currency_ids])]) - res_4 = self.search(cr, uid, [('currency_ids', 'in', [default_company.currency_ids[0].id])]) - res_5 = self.search(cr, uid, [('currency_ids', 'in', default_company.currency_ids[0].id)]) + res_3 = self.search(cr, uid, [('currency_ids', 'in', [x.id for x in default_company.currency_ids])]) # companies having a currency of main company + res_4 = self.search(cr, uid, [('currency_ids', 'in', [default_company.currency_ids[0].id])]) # companies having first currency of main company + res_5 = self.search(cr, uid, [('currency_ids', 'in', default_company.currency_ids[0].id)]) # companies having first currency of main company # res_6 = self.search(cr, uid, [('currency_ids', 'in', [default_company.currency_ids[0].name])]) # TODO res_7 = self.search(cr, uid, [('currency_ids', '=', default_company.currency_ids[0].name)]) res_8 = self.search(cr, uid, [('currency_ids', 'like', default_company.currency_ids[0].name)]) res_9 = self.search(cr, uid, [('currency_ids', 'like', 'probably_unexisting_name')]) # self.search(cr, uid, [('currency_ids', 'unexisting_op', 'probably_unexisting_name')]) # TODO expected exception + assert res_1 == [ref('ymltest_company3')] + assert res_2 == [ref('ymltest_company3')] + assert res_3 == [1] + assert res_4 == [1] + assert res_5 == [1] + assert res_7 == [1] + assert res_8 == [1] + assert res_9 == [] # get the companies referenced by some currency (this is normally the main company) res_10 = self.search(cr, uid, [('currency_ids', 'not like', 'probably_unexisting_name')]) @@ -393,15 +400,7 @@ res_11.sort() res_12.sort() res_13.sort() - assert res_1 == [ref('ymltest_company3')] - assert res_2 == [ref('ymltest_company3')] - assert res_3 == [1] - assert res_4 == [1] - assert res_5 == [1] - assert res_7 == [1] - assert res_8 == [1] - assert res_9 == [] - print ">>> 10:", res_10 + print ">>> 11:", res_11 #assert res_10 == res_11 assert res_10 == res_12 diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 0bb4b739907..deb0af2e951 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -75,10 +75,16 @@ domain is not a valid second-level operand. Unaccent - Accent-insensitive search OpenERP will use the SQL function 'unaccent' when available for the 'ilike' and -'not ilike' operators. Normally the 'unaccent' function is obtained from the -PostgreSQL 'unaccent' contrib module[0]. The steps to install the module might -differ on specific PostgreSQL versions. We give here some instruction for -PostgreSQL 9.x on a Ubuntu system. +'not ilike' operators, and enabled in the configuration. +Normally the 'unaccent' function is obtained from the PostgreSQL 'unaccent' +contrib module[0]. + + +..todo: The following explanation should be moved in some external installation + guide + +The steps to install the module might differ on specific PostgreSQL versions. +We give here some instruction for PostgreSQL 9.x on a Ubuntu system. Ubuntu doesn't come yet with PostgreSQL 9.x, so an alternive package source is used. We use Martin Pitt's PPA available at ppa:pitti/postgresql[1]. See @@ -127,6 +133,7 @@ import openerp.modules NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' +DOMAIN_OPERATORS = (NOT_OPERATOR, OR_OPERATOR, AND_OPERATOR) # List of available term operators. It is also possible to use the '<>' # operator, which is strictly the same as '!='; the later should be prefered @@ -135,13 +142,15 @@ AND_OPERATOR = '&' # only one representation). # An internal (i.e. not available to the user) 'inselect' operator is also # used. In this case its right operand has the form (subselect, params). -OPS = ('=', '!=', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') +TERM_OPERATORS = ('=', '!=', '<=', '<', '>', '>=', '=?', '=like', '=ilike', + 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', + 'child_of') # A subset of the above operators, with a 'negative' semantic. When the -# expressions 'in NEGATIVE_OPS' or 'not in NEGATIVE_OPS' are used in the code -# below, this doesn't necessarily mean that any of those NEGATIVE_OPS is +# expressions 'in NEGATIVE_TERM_OPERATORS' or 'not in NEGATIVE_TERM_OPERATORS' are used in the code +# below, this doesn't necessarily mean that any of those NEGATIVE_TERM_OPERATORS is # legal in the processed term. -NEGATIVE_OPS = ('!=', 'not like', 'not ilike', 'not in') +NEGATIVE_TERM_OPERATORS = ('!=', 'not like', 'not ilike', 'not in') TRUE_LEAF = (1, '=', 1) FALSE_LEAF = (0, '=', 1) @@ -164,7 +173,7 @@ def normalize(domain): op_arity = {NOT_OPERATOR: 1, AND_OPERATOR: 2, OR_OPERATOR: 2} for token in domain: if expected == 0: # more than expected, like in [A, B] - result[0:0] = ['&'] # put an extra '&' in front + result[0:0] = [AND_OPERATOR] # put an extra '&' in front expected = 1 result.append(token) if isinstance(token, (list, tuple)): # domain term @@ -205,16 +214,16 @@ def combine(operator, unit, zero, domains): return result def AND(domains): - """ AND([D1,D2,...]) returns a domain representing D1 and D2 and ... """ + """AND([D1,D2,...]) returns a domain representing D1 and D2 and ... """ return combine(AND_OPERATOR, TRUE_DOMAIN, FALSE_DOMAIN, domains) def OR(domains): - """ OR([D1,D2,...]) returns a domain representing D1 or D2 or ... """ + """OR([D1,D2,...]) returns a domain representing D1 or D2 or ... """ return combine(OR_OPERATOR, FALSE_DOMAIN, TRUE_DOMAIN, domains) def is_operator(element): - """ Test whether an object is a valid domain operator. """ - return isinstance(element, (str, unicode)) and element in [AND_OPERATOR, OR_OPERATOR, NOT_OPERATOR] + """Test whether an object is a valid domain operator. """ + return isinstance(element, basestring) and element in DOMAIN_OPERATORS # TODO change the share wizard to use this function. def is_leaf(element, internal=False): @@ -222,12 +231,11 @@ def is_leaf(element, internal=False): :param internal: allow or not the 'inselect' internal operator in the term. This normally should be always left to False. - """ - INTERNAL_OPS = OPS + ('inselect',) + INTERNAL_OPS = TERM_OPERATORS + ('inselect',) return (isinstance(element, tuple) or isinstance(element, list)) \ and len(element) == 3 \ - and (((not internal) and element[1] in OPS + ('<>',)) \ + and (((not internal) and element[1] in TERM_OPERATORS + ('<>',)) \ or (internal and element[1] in INTERNAL_OPS + ('<>',))) def normalize_leaf(left, operator, right): @@ -247,7 +255,7 @@ def normalize_leaf(left, operator, right): return left, operator, right def distribute_not(domain): - """ Distribute the '!' operator on a normalized domain. + """ Distribute any '!' domain operators found inside a normalized domain. Because we don't use SQL semantic for processing a 'left not in right' query (i.e. our 'not in' is not simply translated to a SQL 'not in'), @@ -256,11 +264,17 @@ def distribute_not(domain): the result with 'not (...)', as it would result in a 'not in' at the SQL level. - This function is thus responsible for pushing the '!' operator inside the - terms. + This function is thus responsible for pushing any '!' domain operators + inside the terms themselves. For example:: + + ['!','&',('user_id','=',4),('partner_id','in',[1,2])] + will be turned into: + ['|',('user_id','!=',4),('partner_id','not in',[1,2])] """ def negate(leaf): + """Negates and returns a single domain leaf term, + using the opposite operator if possible""" left, operator, right = leaf mapping = { '<': '>=', @@ -279,44 +293,50 @@ def distribute_not(domain): if operator in mapping: operator = mapping[operator] return [(left, operator, right)] - return ['!', (left, operator, right)] - def distribute(domain): + return [NOT_OPERATOR, (left, operator, right)] + def distribute_negate(domain): + """Negate the domain ``subtree`` rooted at domain[0], + leaving the rest of the domain intact, and return + (negated_subtree, untouched_domain_rest) + """ if is_leaf(domain[0]): return negate(domain[0]), domain[1:] - if domain[0] == '&': - done1, todo1 = distribute(domain[1:]) - done2, todo2 = distribute(todo1) - return ['|'] + done1 + done2, todo2 - if domain[0] == '|': - done1, todo1 = distribute(domain[1:]) - done2, todo2 = distribute(todo1) - return ['&'] + done1 + done2, todo2 + if domain[0] == AND_OPERATOR: + done1, todo1 = distribute_negate(domain[1:]) + done2, todo2 = distribute_negate(todo1) + return [OR_OPERATOR] + done1 + done2, todo2 + if domain[0] == OR_OPERATOR: + done1, todo1 = distribute_negate(domain[1:]) + done2, todo2 = distribute_negate(todo1) + return [AND_OPERATOR] + done1 + done2, todo2 if not domain: return [] - if domain[0] != '!': + if domain[0] != NOT_OPERATOR: return [domain[0]] + distribute_not(domain[1:]) - if domain[0] == '!': - done, todo = distribute(domain[1:]) + if domain[0] == NOT_OPERATOR: + done, todo = distribute_negate(domain[1:]) return done + distribute_not(todo) -def select_from_where(cr, s, f, w, ids, op): +def select_from_where(cr, select_field, from_table, where_field, where_ids, where_operator): # todo: merge into parent query as sub-query res = [] - if ids: - if op in ['<','>','>=','<=']: + if where_ids: + if where_operator in ['<','>','>=','<=']: cr.execute('SELECT "%s" FROM "%s" WHERE "%s" %s %%s' % \ - (s, f, w, op), (ids[0],)) # TODO shouldn't this be min/max(ids) ? + (select_field, from_table, where_field, where_operator), + (where_ids[0],)) # TODO shouldn't this be min/max(where_ids) ? res = [r[0] for r in cr.fetchall()] - else: # TODO op is supposed to be 'in'? It is called with child_of... - for i in range(0, len(ids), cr.IN_MAX): - subids = ids[i:i+cr.IN_MAX] + else: # TODO where_operator is supposed to be 'in'? It is called with child_of... + for i in range(0, len(where_ids), cr.IN_MAX): + subids = where_ids[i:i+cr.IN_MAX] cr.execute('SELECT "%s" FROM "%s" WHERE "%s" IN %%s' % \ - (s, f, w), (tuple(subids),)) + (select_field, from_table, where_field), (tuple(subids),)) res.extend([r[0] for r in cr.fetchall()]) return res -def select_distinct_from_where_not_null(cr, s, f): - cr.execute('SELECT distinct("%s") FROM "%s" where "%s" is not null' % (s, f, s)) +def select_distinct_from_where_not_null(cr, select_field, from_table): + cr.execute('SELECT distinct("%s") FROM "%s" where "%s" is not null' % \ + (select_field, from_table, select_field)) return [r[0] for r in cr.fetchall()] class expression(object): @@ -342,34 +362,35 @@ class expression(object): return self.__exp[:] def parse(self, cr, uid, exp, table, context): - """ transform the leafs of the expression """ + """ transform the leaves of the expression """ self.__exp = exp self.__main_table = table self.__all_tables.add(table) - def child_of_domain(left, right, table, parent=None, prefix=''): - ids = right - if table._parent_store and (not table.pool._init): -# TODO: Improve where joins are implemented for many with '.', replace by: -# doms += ['&',(prefix+'.parent_left','<',o.parent_right),(prefix+'.parent_left','>=',o.parent_left)] + def child_of_domain(left, ids, left_model, parent=None, prefix=''): + """Returns a domain implementing the child_of operator for [(left,child_of,ids)], + either as a range using the parent_left/right tree lookup fields (when available), + or as an expanded [(left,in,child_ids)]""" + if left_model._parent_store and (not left_model.pool._init): + # TODO: Improve where joins are implemented for many with '.', replace by: + # doms += ['&',(prefix+'.parent_left','<',o.parent_right),(prefix+'.parent_left','>=',o.parent_left)] doms = [] - for o in table.browse(cr, uid, ids, context=context): + for o in left_model.browse(cr, uid, ids, context=context): if doms: doms.insert(0, OR_OPERATOR) doms += [AND_OPERATOR, ('parent_left', '<', o.parent_right), ('parent_left', '>=', o.parent_left)] if prefix: - return [(left, 'in', table.search(cr, uid, doms, context=context))] + return [(left, 'in', left_model.search(cr, uid, doms, context=context))] return doms else: - def rg(ids, table, parent): + def recursive_children(ids, model, parent_field): if not ids: return [] - ids2 = table.search(cr, uid, [(parent, 'in', ids)], context=context) - return ids + rg(ids2, table, parent) - return [(left, 'in', rg(ids, table, parent or table._parent_name))] + ids2 = model.search(cr, uid, [(parent_field, 'in', ids)], context=context) + return ids + recursive_children(ids2, model, parent_field) + return [(left, 'in', recursive_children(ids, left_model, parent or left_model._parent_name))] - # TODO rename this function as it is not strictly for 'child_of', but also for 'in'... - def child_of_right_to_ids(value, field_obj): + def to_ids(value, field_obj): """ Normalize a single id, or a string, or a list of ids to a list of ids. """ if isinstance(value, basestring): @@ -417,7 +438,7 @@ class expression(object): if not field: if left == 'id' and operator == 'child_of': - ids2 = child_of_right_to_ids(right, table) + ids2 = to_ids(right, table) dom = child_of_domain(left, ids2, working_table) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] continue @@ -458,11 +479,10 @@ class expression(object): elif field._type == 'one2many': # Applying recursivity on field(one2many) if operator == 'child_of': + ids2 = to_ids(right, field_obj) if field._obj != working_table._name: - ids2 = child_of_right_to_ids(right, field_obj) dom = child_of_domain(left, ids2, field_obj, prefix=field._obj) else: - ids2 = child_of_right_to_ids(right, field_obj) dom = child_of_domain('id', ids2, working_table, parent=left) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] @@ -486,11 +506,11 @@ class expression(object): self.__exp[i] = FALSE_LEAF else: call_null = False - o2m_op = 'not in' if operator in NEGATIVE_OPS else 'in' + o2m_op = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in' self.__exp[i] = ('id', o2m_op, select_from_where(cr, field._fields_id, field_obj._table, 'id', ids2, operator)) if call_null: - o2m_op = 'in' if operator in NEGATIVE_OPS else 'not in' + o2m_op = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in' self.__exp[i] = ('id', o2m_op, select_distinct_from_where_not_null(cr, field._fields_id, field_obj._table)) elif field._type == 'many2many': @@ -501,7 +521,7 @@ class expression(object): return ids return select_from_where(cr, field._id1, field._rel, field._id2, ids, operator) - ids2 = child_of_right_to_ids(right, field_obj) + ids2 = to_ids(right, field_obj) dom = child_of_domain('id', ids2, field_obj) ids2 = field_obj.search(cr, uid, dom, context=context) self.__exp[i] = ('id', 'in', _rec_convert(ids2)) @@ -526,16 +546,16 @@ class expression(object): operator = 'in' # operator changed because ids are directly related to main object else: call_null_m2m = False - m2m_op = 'not in' if operator in NEGATIVE_OPS else 'in' + m2m_op = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in' self.__exp[i] = ('id', m2m_op, select_from_where(cr, field._id1, field._rel, field._id2, res_ids, operator) or [0]) if call_null_m2m: - m2m_op = 'in' if operator in NEGATIVE_OPS else 'not in' + m2m_op = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in' self.__exp[i] = ('id', m2m_op, select_distinct_from_where_not_null(cr, field._id1, field._rel)) elif field._type == 'many2one': if operator == 'child_of': - ids2 = child_of_right_to_ids(right, field_obj) + ids2 = to_ids(right, field_obj) if field._obj != working_table._name: dom = child_of_domain(left, ids2, field_obj, prefix=field._obj) else: @@ -558,7 +578,7 @@ class expression(object): elif isinstance(right, list) and operator in ['!=','=']: #for domain (FIELD,'=',['value1','value2']) operator = dict_op[operator] res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, limit=None, context=c)] - if operator in NEGATIVE_OPS: + if operator in NEGATIVE_TERM_OPERATORS: res_ids.append(False) # TODO this should not be appended if False was in 'right' return (left, 'in', res_ids) From 9497814729f37407f3c3539018a1df116d23087f Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Thu, 22 Sep 2011 02:54:31 +0200 Subject: [PATCH 24/24] [IMP] expression: fix [(o2m,not in,...)] domains, re-enable tests, cleanup bzr revid: odo@openerp.com-20110922005431-gnym5pormtjkfu8q --- .../addons/base/test/test_osv_expression.yml | 30 +++++++++---------- openerp/osv/expression.py | 21 ++++++++----- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 5ecab625911..97ab6c9e83c 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -186,21 +186,23 @@ partner_ids = self.search(cr, uid, []) partner_ids.sort() max_partner_id = max(partner_ids) + + # Grab test sample data without using a normal + # search domain, because we want to test these later, + # so we can't rely on them! partners = self.browse(cr, uid, partner_ids) with_parent = [] without_parent = [] - for x in partners: - if x.parent_id and x.parent_id.id in partner_ids: - with_parent.append(x.id) - with_parent.sort() - for x in partners: - if not x.parent_id: - without_parent.append(x.id) - without_parent.sort() with_website = [] for x in partners: + if x.parent_id: + with_parent.append(x.id) + else: + without_parent.append(x.id) if x.website: with_website.append(x.id) + with_parent.sort() + without_parent.sort() with_website.sort() # We treat null values differently than in SQL. For instance in SQL: @@ -400,13 +402,9 @@ res_11.sort() res_12.sort() res_13.sort() - - print ">>> 11:", res_11 - #assert res_10 == res_11 + assert res_10 == res_11 assert res_10 == res_12 - print ">>> 12:", res_12 assert res_10 == res_13 - print ">>> 13:", res_13 # child_of x returns x and its children (direct or not). company = self.browse(cr, uid, ref('ymltest_company3')) @@ -416,13 +414,13 @@ res_1.sort() res_2 = self.search(cr, uid, [('id', 'child_of', ref('ymltest_company3'))]) res_2.sort() - #res_3 = self.search(cr, uid, [('id', 'child_of', [company.name])]) # TODO - #res_3.sort() + res_3 = self.search(cr, uid, [('id', 'child_of', [company.name])]) + res_3.sort() res_4 = self.search(cr, uid, [('id', 'child_of', company.name)]) res_4.sort() assert res_1 == expected assert res_2 == expected - #assert res_3 == expected + assert res_3 == expected assert res_4 == expected - Verify that normalize_domain() works. diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 4d558bdb619..db856381e6b 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -391,14 +391,18 @@ class expression(object): return [(left, 'in', recursive_children(ids, left_model, parent or left_model._parent_name))] def to_ids(value, field_obj): - """ Normalize a single id, or a string, or a list of ids to a list of ids. - """ + """Normalize a single id or name, or a list of those, into a list of ids""" + names = [] if isinstance(value, basestring): - return [x[0] for x in field_obj.name_search(cr, uid, value, [], 'ilike', context=context, limit=None)] + names = [value] + if value and isinstance(value, (tuple, list)) and isinstance(value[0], basestring): + names = value + if names: + return flatten([[x[0] for x in field_obj.name_search(cr, uid, n, [], 'ilike', context=context, limit=None)] \ + for n in names]) elif isinstance(value, (int, long)): return [value] - else: - return list(value) + return list(value) i = -1 while i + 1