[FIX] core: avoid NULLs in subselects generated for one2many fields
This fixes a problem that occurs with conditions on one2many fields,
where most conditions are translated with a subquery like
"model".id IN (
SELECT "comodel"."model_id" FROM "comodel" WHERE ...
)
The issue occurs when the subquery return NULL values, i.e., when the
field "model_id" in the subquery above can be NULL. The fix simply
consists in adding the condition "model_id IS NOT NULL" in the WHERE
clause of the subquery.
The problem with NULL values returned by subqueries is the fact that
they make the SQL condition undetermined instead of false, and this
discards some of the results returned by a query. Simply consider:
id NOT IN (1, 2, 3)
The value id=3 makes the condition false, while i=4 makes it true. Now
consider that the set also includes a NULL value, like:
id NOT IN (1, 2, 3, NULL)
The value id=3 makes the condition false, but the value id=4 makes it
undetermined. Therefore this condition is actually undetermined for all
possible values of id, and a query containing that condition simply
returns nothing!
closes odoo/odoo#87285
X-original-commit: 6d680bac0333c6c7ce9f056cba0e1f7e9fa22f9c
Signed-off-by: Vincent Schippefilt (vsc) <vsc@odoo.com>
Signed-off-by: Raphael Collet <rco@odoo.com>
This commit is contained in:
@@ -1496,7 +1496,7 @@ class TestOne2many(TransactionCase):
|
||||
SELECT "res_partner_bank"."partner_id"
|
||||
FROM "res_partner_bank"
|
||||
WHERE ("res_partner_bank"."sanitized_acc_number"::text LIKE %s)
|
||||
))
|
||||
)) AND "res_partner"."parent_id" IS NOT NULL
|
||||
))
|
||||
ORDER BY "res_partner"."display_name", "res_partner"."id"
|
||||
''']):
|
||||
@@ -1637,6 +1637,30 @@ class TestOne2many(TransactionCase):
|
||||
''']):
|
||||
self.Partner.search([('bank_ids', 'like', '12')])
|
||||
|
||||
def test_empty(self):
|
||||
self.Partner.search([('bank_ids', '!=', False)], order='id')
|
||||
self.Partner.search([('bank_ids', '=', False)], order='id')
|
||||
|
||||
with self.assertQueries(['''
|
||||
SELECT "res_partner".id
|
||||
FROM "res_partner"
|
||||
WHERE ("res_partner"."id" IN (
|
||||
SELECT "partner_id" FROM "res_partner_bank" WHERE "partner_id" IS NOT NULL
|
||||
))
|
||||
ORDER BY "res_partner"."id"
|
||||
''']):
|
||||
self.Partner.search([('bank_ids', '!=', False)], order='id')
|
||||
|
||||
with self.assertQueries(['''
|
||||
SELECT "res_partner".id
|
||||
FROM "res_partner"
|
||||
WHERE ("res_partner"."id" NOT IN (
|
||||
SELECT "partner_id" FROM "res_partner_bank" WHERE "partner_id" IS NOT NULL
|
||||
))
|
||||
ORDER BY "res_partner"."id"
|
||||
''']):
|
||||
self.Partner.search([('bank_ids', '=', False)], order='id')
|
||||
|
||||
|
||||
class TestMany2many(TransactionCase):
|
||||
def setUp(self):
|
||||
|
||||
@@ -2844,12 +2844,12 @@ class TestX2many(common.TransactionCase):
|
||||
result = recs.search([('id', 'in', recs.ids), ('lines', 'not in', [])])
|
||||
self.assertEqual(result, recs)
|
||||
|
||||
# these cases are weird
|
||||
# test 'not in' where the lines contain NULL values
|
||||
result = recs.search([('id', 'in', recs.ids), ('lines', 'not in', (line1 + line0).ids)])
|
||||
self.assertEqual(result, recs.browse())
|
||||
self.assertEqual(result, recs - recX)
|
||||
|
||||
result = recs.search([('id', 'in', recs.ids), ('lines', 'not in', line0.ids)])
|
||||
self.assertEqual(result, recs.browse())
|
||||
self.assertEqual(result, recs)
|
||||
|
||||
# special case: compare with False
|
||||
result = recs.search([('id', 'in', recs.ids), ('lines', '=', False)])
|
||||
|
||||
+23
-15
@@ -765,7 +765,8 @@ class expression(object):
|
||||
|
||||
elif field.type == 'one2many':
|
||||
domain = field.get_domain_list(model)
|
||||
inverse_is_int = comodel._fields[field.inverse_name].type in ('integer', 'many2one_reference')
|
||||
inverse_field = comodel._fields[field.inverse_name]
|
||||
inverse_is_int = inverse_field.type in ('integer', 'many2one_reference')
|
||||
unwrap_inverse = (lambda ids: ids) if inverse_is_int else (lambda recs: recs.ids)
|
||||
|
||||
if right is not False:
|
||||
@@ -781,36 +782,43 @@ class expression(object):
|
||||
if inverse_is_int and domain:
|
||||
ids2 = comodel._search([('id', 'in', ids2)] + domain, order='id')
|
||||
|
||||
if isinstance(ids2, Query) and comodel._fields[field.inverse_name].store:
|
||||
op1 = 'not inselect' if operator in NEGATIVE_TERM_OPERATORS else 'inselect'
|
||||
subquery, subparams = ids2.subselect('"%s"."%s"' % (comodel._table, field.inverse_name))
|
||||
push(('id', op1, (subquery, subparams)), model, alias, internal=True)
|
||||
elif ids2 and comodel._fields[field.inverse_name].store:
|
||||
op1 = 'not inselect' if operator in NEGATIVE_TERM_OPERATORS else 'inselect'
|
||||
subquery = 'SELECT "%s" FROM "%s" WHERE "id" IN %%s' % (field.inverse_name, comodel._table)
|
||||
subparams = [tuple(ids2)]
|
||||
push(('id', op1, (subquery, subparams)), model, alias, internal=True)
|
||||
if inverse_field.store:
|
||||
# In the condition, one must avoid subqueries to return
|
||||
# NULL values, since it makes the IN test NULL instead
|
||||
# of FALSE. This may discard expected results, as for
|
||||
# instance "id NOT IN (42, NULL)" is never TRUE.
|
||||
in_ = 'NOT IN' if operator in NEGATIVE_TERM_OPERATORS else 'IN'
|
||||
if isinstance(ids2, Query):
|
||||
if not inverse_field.required:
|
||||
ids2.add_where(f'"{comodel._table}"."{inverse_field.name}" IS NOT NULL')
|
||||
subquery, subparams = ids2.subselect(f'"{comodel._table}"."{inverse_field.name}"')
|
||||
else:
|
||||
subquery = f'SELECT "{inverse_field.name}" FROM "{comodel._table}" WHERE "id" IN %s'
|
||||
if not inverse_field.required:
|
||||
subquery += f' AND "{inverse_field.name}" IS NOT NULL'
|
||||
subparams = [tuple(ids2) or (None,)]
|
||||
push_result(f'("{alias}"."id" {in_} ({subquery}))', subparams)
|
||||
else:
|
||||
# determine ids1 in model related to ids2
|
||||
recs = comodel.browse(ids2).sudo().with_context(prefetch_fields=False)
|
||||
ids1 = unwrap_inverse(recs.mapped(field.inverse_name))
|
||||
ids1 = unwrap_inverse(recs.mapped(inverse_field.name))
|
||||
# rewrite condition in terms of ids1
|
||||
op1 = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in'
|
||||
push(('id', op1, ids1), model, alias)
|
||||
|
||||
else:
|
||||
if comodel._fields[field.inverse_name].store and not (inverse_is_int and domain):
|
||||
if inverse_field.store and not (inverse_is_int and domain):
|
||||
# rewrite condition to match records with/without lines
|
||||
op1 = 'inselect' if operator in NEGATIVE_TERM_OPERATORS else 'not inselect'
|
||||
subquery = 'SELECT "%s" FROM "%s" where "%s" is not null' % (field.inverse_name, comodel._table, field.inverse_name)
|
||||
subquery = f'SELECT "{inverse_field.name}" FROM "{comodel._table}" WHERE "{inverse_field.name}" IS NOT NULL'
|
||||
push(('id', op1, (subquery, [])), model, alias, internal=True)
|
||||
else:
|
||||
comodel_domain = [(field.inverse_name, '!=', False)]
|
||||
comodel_domain = [(inverse_field.name, '!=', False)]
|
||||
if inverse_is_int and domain:
|
||||
comodel_domain += domain
|
||||
recs = comodel.search(comodel_domain, order='id').sudo().with_context(prefetch_fields=False)
|
||||
# determine ids1 = records with lines
|
||||
ids1 = unwrap_inverse(recs.mapped(field.inverse_name))
|
||||
ids1 = unwrap_inverse(recs.mapped(inverse_field.name))
|
||||
# rewrite condition to match records with/without lines
|
||||
op1 = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in'
|
||||
push(('id', op1, ids1), model, alias)
|
||||
|
||||
Reference in New Issue
Block a user