odoo
/
odoo
2
0
Fork 0
Code Pull Requests Releases Activity

[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
This commit is contained in:
Vo Minh Thu 2011-08-10 10:56:04 +02:00
parent 64346c63a4
commit 44b3499a73
2 changed files with 59 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
openerp/addons/base/test/test_osv_expression.yml
View File

@ -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).
-

74
openerp/osv/expression.py
View File

@ -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)