diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG index fdeaff06fd..f537843802 100644 --- a/trytond/CHANGELOG +++ b/trytond/CHANGELOG @@ -1,3 +1,4 @@ +* Use EXISTS for search on O2M with many records * Replace target_search by use of count() * Manage domain on id in single value list as unique * Do not validate domain for empty fields diff --git a/trytond/trytond/model/fields/one2many.py b/trytond/trytond/model/fields/one2many.py index 711202988f..d71ba49d7b 100644 --- a/trytond/trytond/model/fields/one2many.py +++ b/trytond/trytond/model/fields/one2many.py @@ -3,8 +3,11 @@ from collections import defaultdict from itertools import chain +from sql import Literal from sql.conditionals import Coalesce +from sql.operators import Exists +from trytond.config import config from trytond.pool import Pool from trytond.pyson import PYSONEncoder from trytond.tools import cached_property, grouped_slice @@ -16,6 +19,8 @@ from .field import ( size_validate) from .function import Function +_subquery_threshold = config.getint('database', 'subquery_threshold') + class One2Many(Field): ''' @@ -332,20 +337,42 @@ class One2Many(Field): origin_where = origin.like(Model.__name__ + ',%') origin = origin_field.sql_id(origin, Target) + use_in = Target.count() < _subquery_threshold if '.' not in name: if value is None: - where = origin != value - if history_where: - where &= history_where - if origin_where: - where &= origin_where - if self.filter: - query = Target.search(self.filter, order=[], query=True) - where &= origin.in_(query) - query = target.select(origin, where=where) - expression = ~table.id.in_(query) + if use_in: + where = origin != value + if history_where: + where &= history_where + if origin_where: + where &= origin_where + if self.filter: + query = Target.search( + self.filter, order=[], query=True) + where &= origin.in_(query) + query = target.select(origin, where=where) + expression = ~table.id.in_(query) + else: + where = origin == table.id + if history_where: + where &= history_where + if origin_where: + where &= origin_where + if self.filter: + target_tables = { + None: (target, None), + } + target_tables, clause = Target.search_domain( + self.filter, tables=target_tables) + where &= clause + query_table = convert_from(None, target_tables) + query = query_table.select(Literal(1), where=where) + else: + query = target.select(Literal(1), where=where) + expression = ~Exists(query) + if operator == '!=': - return ~expression + expression = ~expression return expression else: if isinstance(value, str): @@ -372,13 +399,22 @@ class One2Many(Field): tables, expression = Target.search_domain( target_domain, tables=target_tables) query_table = convert_from(None, target_tables) - query = query_table.select(origin, where=expression) - expression = table.id.in_(query) + if use_in: + query = query_table.select(origin, where=expression) + expression = table.id.in_(query) + else: + query = query_table.select( + Literal(1), where=expression & (origin == table.id)) + expression = Exists(query) if operator == 'not where': expression = ~expression elif operator.startswith('!') or operator.startswith('not '): - expression |= ~table.id.in_(target.select(origin)) + if use_in: + expression |= ~table.id.in_(target.select(origin)) + else: + expression |= ~Exists(target.select( + Literal(1), where=origin == table.id)) return expression def definition(self, model, language): diff --git a/trytond/trytond/tests/test_field_one2many.py b/trytond/trytond/tests/test_field_one2many.py index 922288b55f..a22407d13b 100644 --- a/trytond/trytond/tests/test_field_one2many.py +++ b/trytond/trytond/tests/test_field_one2many.py @@ -2,90 +2,14 @@ # this repository contains the full copyright notices and license terms. import unittest +from trytond.config import config from trytond.model.exceptions import ( RequiredValidationError, SizeValidationError) from trytond.pool import Pool from trytond.tests.test_tryton import activate_module, with_transaction -class CommonTestCaseMixin: - - @with_transaction() - def test_set_reverse_field_instance(self): - "Test reverse field is set on instance" - One2Many = self.One2Many() - Target = self.One2ManyTarget() - - record = One2Many() - target = Target() - record.targets = [target] - - self.assertEqual(target.origin, record) - - @with_transaction() - def test_save_reverse_field(self): - "Test save with reverse field set" - One2Many = self.One2Many() - Target = self.One2ManyTarget() - - record = One2Many() - target = Target() - record.targets = [target] - - record.save() - - @with_transaction() - def test_save_reverse_field_saved(self): - "Test save with reverse field set on saved" - One2Many = self.One2Many() - Target = self.One2ManyTarget() - - record = One2Many() - target = Target() - target.save() - record.targets = [target] - - record.save() - - @with_transaction() - def test_set_reverse_field_dict(self): - "Test reverse field is set on dict" - One2Many = self.One2Many() - - record = One2Many() - record.targets = [{}] - target, = record.targets - - self.assertEqual(target.origin, record) - - @with_transaction() - def test_set_reverse_field_id(self): - "Test reverse field is set on id" - One2Many = self.One2Many() - Target = self.One2ManyTarget() - - record = One2Many() - target = Target() - target.save() - record.targets = [target.id] - target, = record.targets - - self.assertEqual(target.origin, record) - - @with_transaction() - def test_create(self): - "Test create one2many" - One2Many = self.One2Many() - - one2many, = One2Many.create([{ - 'targets': [ - ('create', [{ - 'name': "Target", - }]), - ], - }]) - - self.assertEqual(len(one2many.targets), 1) +class SearchTestCaseMixin: @with_transaction() def test_search_equals(self): @@ -320,6 +244,98 @@ class CommonTestCaseMixin: self.assertListEqual(one2manys, []) + @with_transaction() + def test_search_strategy(self): + "Test search one2many with the right strategy" + One2Many = self.One2Many() + one2many, = One2Many.create([{ + 'targets': [('create', [{'name': "Target"}])], + }]) + + One2Many.search([('targets.name', '=', "Target")]) + query = One2Many.search([('targets.name', '=', "Target")], query=True) + self.assertIn(self._strategy, str(query)) + + +class CommonTestCaseMixin: + + @with_transaction() + def test_set_reverse_field_instance(self): + "Test reverse field is set on instance" + One2Many = self.One2Many() + Target = self.One2ManyTarget() + + record = One2Many() + target = Target() + record.targets = [target] + + self.assertEqual(target.origin, record) + + @with_transaction() + def test_save_reverse_field(self): + "Test save with reverse field set" + One2Many = self.One2Many() + Target = self.One2ManyTarget() + + record = One2Many() + target = Target() + record.targets = [target] + + record.save() + + @with_transaction() + def test_save_reverse_field_saved(self): + "Test save with reverse field set on saved" + One2Many = self.One2Many() + Target = self.One2ManyTarget() + + record = One2Many() + target = Target() + target.save() + record.targets = [target] + + record.save() + + @with_transaction() + def test_set_reverse_field_dict(self): + "Test reverse field is set on dict" + One2Many = self.One2Many() + + record = One2Many() + record.targets = [{}] + target, = record.targets + + self.assertEqual(target.origin, record) + + @with_transaction() + def test_set_reverse_field_id(self): + "Test reverse field is set on id" + One2Many = self.One2Many() + Target = self.One2ManyTarget() + + record = One2Many() + target = Target() + target.save() + record.targets = [target.id] + target, = record.targets + + self.assertEqual(target.origin, record) + + @with_transaction() + def test_create(self): + "Test create one2many" + One2Many = self.One2Many() + + one2many, = One2Many.create([{ + 'targets': [ + ('create', [{ + 'name': "Target", + }]), + ], + }]) + + self.assertEqual(len(one2many.targets), 1) + @with_transaction() def test_write_write(self): "Test write one2many write" @@ -433,8 +449,10 @@ class CommonTestCaseMixin: self.assertIsNone(target.write_date) -class FieldOne2ManyTestCase(unittest.TestCase, CommonTestCaseMixin): +class FieldOne2ManyTestCase( + unittest.TestCase, CommonTestCaseMixin, SearchTestCaseMixin): "Test Field One2Many" + _strategy = 'IN' @classmethod def setUpClass(cls): @@ -646,8 +664,10 @@ class FieldOne2ManyTestCase(unittest.TestCase, CommonTestCaseMixin): self.assertEqual(record.targets[0].context, record.id) -class FieldOne2ManyReferenceTestCase(unittest.TestCase, CommonTestCaseMixin): +class FieldOne2ManyReferenceTestCase( + unittest.TestCase, CommonTestCaseMixin, SearchTestCaseMixin): "Test Field One2Many Reference" + _strategy = 'IN' @classmethod def setUpClass(cls): @@ -658,3 +678,53 @@ class FieldOne2ManyReferenceTestCase(unittest.TestCase, CommonTestCaseMixin): def One2ManyTarget(self): return Pool().get('test.one2many_reference.target') + + +class FieldOne2ManyExistsTestCase(unittest.TestCase, SearchTestCaseMixin): + "Test Field One2Many when using EXISTS" + _strategy = 'EXISTS' + + @classmethod + def setUpClass(cls): + activate_module('tests') + + def setUp(self): + from trytond.model.fields import one2many + super().setUp() + previous = int( + config.get('database', 'subquery_threshold', default='1_000')) + one2many._subquery_threshold = 0 + self.addCleanup(setattr, one2many, '_subquery_threshold', previous) + + def One2Many(self): + return Pool().get('test.one2many') + + def One2ManyTarget(self): + return Pool().get('test.one2many.target') + + +class FieldOne2ManyReferenceExistsTestCase( + unittest.TestCase, SearchTestCaseMixin): + "Test Field One2Many Reference when using EXISTS" + _strategy = 'EXISTS' + + @classmethod + def setUpClass(cls): + activate_module('tests') + + def setUp(self): + from trytond.model.fields import one2many + super().setUp() + previous = int( + config.get('database', 'subquery_threshold', default='1_000')) + one2many._subquery_threshold = 0 + self.addCleanup(setattr, one2many, '_subquery_threshold', previous) + + def One2Many(self): + return Pool().get('test.one2many_reference') + + def One2ManyTarget(self): + return Pool().get('test.one2many_reference.target') + + def assert_strategy(self, query): + self.assertIn('EXISTS', query)