Odoo Server (MOVED TO GITHUB)

Merge lp:~therp-nl/openobject-server/7.0-lp1062795-search_m2m_negative_gives_wrong_results into lp:openobject-server

  1. 7.0-lp1062795-search_m2m_negative_gives_wrong_results
  2. Merge into trunk
Proposed by Stefan Rijnhart (Opener)
Status: Needs review
Proposed branch: lp:~therp-nl/openobject-server/7.0-lp1062795-search_m2m_negative_gives_wrong_results
Merge into: lp:openobject-server
Diff against target: 120 lines (+30/-26)
2 files modified
openerp/addons/base/tests/test_expression.py (+6/-0)
openerp/osv/expression.py (+24/-26)
To merge this branch: bzr merge lp:~therp-nl/openobject-server/7.0-lp1062795-search_m2m_negative_gives_wrong_results
Reviewer Review Type Date Requested Status
OpenERP Core Team Pending
Review via email: mp+131632@code.launchpad.net

This proposal supersedes a proposal from 2012-10-06.

To post a comment you must log in.

Unmerged revisions

4514. By Stefan Rijnhart (Opener)

[FIX] Preserve negative search operator's exclusivity in *2m dotted subqueries
[FIX] Search on *2many with negative operator gives wrong results

4513. By Stefan Rijnhart (Opener)

[ADD] tests for lp:1062795

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'openerp/addons/base/tests/test_expression.py'
2--- openerp/addons/base/tests/test_expression.py 2012-10-26 11:01:09 +0000
3+++ openerp/addons/base/tests/test_expression.py 2012-10-26 14:58:22 +0000
4@@ -65,6 +65,12 @@
5 self.assertTrue(ab not in without_b, "Search for category_id doesn't contain cat_b failed (2).")
6 self.assertTrue(set([a, c]).issubset(set(without_b)), "Search for category_id doesn't contain cat_b failed (3).")
7
8+ # We can exclude any partner containing the category A by string comparison.
9+ without_a = partners.search(cr, uid, [('category_id', '!=', 'test_expression_category_A')])
10+ self.assertTrue(a not in without_a, "Search for category_id is not cat_a by name failed (1).")
11+ self.assertTrue(ab not in without_a, "Search for category_id is not cat_a by name failed (2).")
12+ self.assertTrue(set([b, c]).issubset(set(without_a)), "Search for category_id is not cat_a by name failed (3).")
13+
14 # We can't express the following: Partners with a category different than A.
15 # with_any_other_than_a = ...
16 # self.assertTrue(a not in with_any_other_than_a, "Search for category_id with any other than cat_a failed (1).")
17
18=== modified file 'openerp/osv/expression.py'
19--- openerp/osv/expression.py 2012-10-18 12:47:50 +0000
20+++ openerp/osv/expression.py 2012-10-26 14:58:22 +0000
21@@ -165,6 +165,11 @@
22 # below, this doesn't necessarily mean that any of those NEGATIVE_TERM_OPERATORS is
23 # legal in the processed term.
24 NEGATIVE_TERM_OPERATORS = ('!=', 'not like', 'not ilike', 'not in')
25+REVERSE_NEGATIVE_OPERATOR = {'!=': '=', 'not like': 'like', 'not ilike': 'ilike', 'not in': 'in'}
26+def get_ids_operator(operator):
27+ if operator in NEGATIVE_TERM_OPERATORS:
28+ return 'not in'
29+ return 'in'
30
31 TRUE_LEAF = (1, '=', 1)
32 FALSE_LEAF = (0, '=', 1)
33@@ -475,8 +480,10 @@
34 self.__exp[i] = (field_path[0], 'in', right)
35 # Making search easier when there is a left operand as field.o2m or field.m2m
36 if field._type in ['many2many', 'one2many']:
37- right = field_obj.search(cr, uid, [(field_path[1], operator, right)], context=context)
38- right1 = table.search(cr, uid, [(field_path[0],'in', right)], context=dict(context, active_test=False))
39+ local_operator = REVERSE_NEGATIVE_OPERATOR.get(operator, operator)
40+ right = field_obj.search(cr, uid, [(field_path[1], local_operator, right)], context=context)
41+ ids_operator = get_ids_operator(operator)
42+ right1 = table.search(cr, uid, [(field_path[0], ids_operator, right)], context=dict(context, active_test=False))
43 self.__exp[i] = ('id', 'in', right1)
44
45 if not isinstance(field, fields.property):
46@@ -519,13 +526,11 @@
47 self.__exp = self.__exp[:i] + dom + self.__exp[i+1:]
48
49 else:
50- call_null = True
51-
52 if right is not False:
53 if isinstance(right, basestring):
54- ids2 = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, context=context, limit=None)]
55- if ids2:
56- operator = 'in'
57+ local_operator = REVERSE_NEGATIVE_OPERATOR.get(operator, operator)
58+ operator = get_ids_operator(operator)
59+ ids2 = [x[0] for x in field_obj.name_search(cr, uid, right, [], local_operator, context=context, limit=None)]
60 else:
61 if not isinstance(right, list):
62 ids2 = [right]
63@@ -534,16 +539,13 @@
64 if not ids2:
65 if operator in ['like','ilike','in','=']:
66 #no result found with given search criteria
67- call_null = False
68 self.__exp[i] = FALSE_LEAF
69+ else:
70+ self.__exp[i] = TRUE_LEAF
71 else:
72- ids2 = select_from_where(cr, field._fields_id, field_obj._table, 'id', ids2, operator)
73- if ids2:
74- call_null = False
75- o2m_op = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in'
76- self.__exp[i] = ('id', o2m_op, ids2)
77-
78- if call_null:
79+ ids2 = select_from_where(cr, field._fields_id, field_obj._table, 'id', ids2, "in")
80+ self.__exp[i] = ('id', operator, ids2)
81+ else:
82 o2m_op = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in'
83 self.__exp[i] = ('id', o2m_op, select_distinct_from_where_not_null(cr, field._fields_id, field_obj._table))
84
85@@ -561,12 +563,11 @@
86 ids2 = field_obj.search(cr, uid, dom, context=context)
87 self.__exp[i] = ('id', 'in', _rec_convert(ids2))
88 else:
89- call_null_m2m = True
90 if right is not False:
91 if isinstance(right, basestring):
92- res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], operator, context=context)]
93- if res_ids:
94- operator = 'in'
95+ local_operator = REVERSE_NEGATIVE_OPERATOR.get(operator, operator)
96+ operator = get_ids_operator(operator)
97+ res_ids = [x[0] for x in field_obj.name_search(cr, uid, right, [], local_operator, context=context)]
98 else:
99 if not isinstance(right, list):
100 res_ids = [right]
101@@ -575,16 +576,13 @@
102 if not res_ids:
103 if operator in ['like','ilike','in','=']:
104 #no result found with given search criteria
105- call_null_m2m = False
106 self.__exp[i] = FALSE_LEAF
107 else:
108- operator = 'in' # operator changed because ids are directly related to main object
109+ self.__exp[i] = TRUE_LEAF
110 else:
111- call_null_m2m = False
112- m2m_op = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in'
113- self.__exp[i] = ('id', m2m_op, select_from_where(cr, rel_id1, rel_table, rel_id2, res_ids, operator) or [0])
114-
115- if call_null_m2m:
116+ m2m_op = get_ids_operator(operator)
117+ self.__exp[i] = ('id', m2m_op, select_from_where(cr, rel_id1, rel_table, rel_id2, res_ids, "in") or [0])
118+ else:
119 m2m_op = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in'
120 self.__exp[i] = ('id', m2m_op, select_distinct_from_where_not_null(cr, rel_id1, rel_table))
121