From 0c4725b8914b65d4f18ced6903819ec4cdd5555c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Mon, 15 Jun 2015 01:10:59 +0200 Subject: [PATCH] Add target_search option on Many2One The option allow to select between the subquery or the join format for target search clause. issue4798 review16341002 --- CHANGELOG | 2 ++ doc/ref/models/fields.rst | 35 ++++++++++++++++------------- doc/ref/models/models.rst | 21 ++++++++++++------ trytond/model/fields/many2one.py | 38 +++++++++++++++++++++++++------- trytond/model/modelsql.py | 9 ++++---- trytond/tests/__init__.py | 1 + trytond/tests/test.py | 7 ++++++ trytond/tests/test_fields.py | 25 +++++++++++++++++++++ 8 files changed, 104 insertions(+), 34 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 104aaae65d..ed5db091a4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,5 @@ +* Add target_search option to Many2One +* Add tables argument to ModelSQL.search_domain * Rename ir.module.module* into ir.module* Version 3.6.0 - 2015-04-20 diff --git a/doc/ref/models/fields.rst b/doc/ref/models/fields.rst index 18aa231978..032603ea35 100644 --- a/doc/ref/models/fields.rst +++ b/doc/ref/models/fields.rst @@ -155,20 +155,8 @@ Instance methods: .. method:: Field.convert_domain(domain, tables, Model) Convert the simple :ref:`domain ` clause into a SQL - expression or a new domain. - -Where ``tables`` is a nested dictionary containing the existing joins (and it -could be updated to add new joins):: - - { - None: (, None), - 'party': { - None: (
, ), - 'addresses': { - None: (
, ), - }, - }, - } + expression or a new domain. :ref:`tables ` could be updated to + add new joins. .. method:: Field.sql_format(value) @@ -478,7 +466,7 @@ Same like :attr:`Selection.selection_change_with`. Many2One -------- -.. class:: Many2One(model_name, string[, left[, right[, ondelete[, datetime_field[, \**options]]]]]) +.. class:: Many2One(model_name, string[, left[, right[, ondelete[, datetime_field[, target_search[, \**options]]]]]) A many-to-one relation field. @@ -529,6 +517,23 @@ A many-to-one relation field. :attr:`trytond.model.ModelSQL._history` to request a value for a given date and time on a historicized model. +.. attribute:: Many2One.target_search + + Define the kind of SQL query to use when searching on related target. + Allowed values are: + + - ``subquery``: it will use a subquery based on the ids. + + - ``join``: it will add a join on the main query. + + ``join`` is the default value. + + .. note:: + + ``join`` could improve the performance if the target has a huge amount + of records. + .. + One2Many -------- diff --git a/doc/ref/models/models.rst b/doc/ref/models/models.rst index 26722d461a..caa32ce796 100644 --- a/doc/ref/models/models.rst +++ b/doc/ref/models/models.rst @@ -452,17 +452,24 @@ Class methods: Return a list of records that match the :ref:`domain ` or the sql query if query is True. -.. classmethod:: ModelSQL.search_domain(domain[, active_test]) +.. classmethod:: ModelSQL.search_domain(domain[, active_test[, tables]]) - Convert a :ref:`domain ` into a tuple containing: + Convert a :ref:`domain ` into a SQL expression by returning + the updated tables dictionary and a SQL expression. - - a SQL clause string +.. _ref-tables: - - a list of arguments for the SQL clause + Where ``tables`` is a nested dictionary containing the existing joins:: - - a list of tables used in the SQL clause - - - a list of arguments for the tables + { + None: (
, None), + 'party': { + None: (
, ), + 'addresses': { + None: (
, ), + }, + }, + } ======== Workflow diff --git a/trytond/model/fields/many2one.py b/trytond/model/fields/many2one.py index a7051a5d3e..6fd9c5106c 100644 --- a/trytond/model/fields/many2one.py +++ b/trytond/model/fields/many2one.py @@ -18,9 +18,9 @@ class Many2One(Field): _type = 'many2one' def __init__(self, model_name, string='', left=None, right=None, - ondelete='SET NULL', datetime_field=None, help='', required=False, - readonly=False, domain=None, states=None, select=False, - on_change=None, on_change_with=None, depends=None, + ondelete='SET NULL', datetime_field=None, target_search='join', + help='', required=False, readonly=False, domain=None, states=None, + select=False, on_change=None, on_change_with=None, depends=None, context=None, loading='eager'): ''' :param model_name: The name of the target model. @@ -33,6 +33,7 @@ class Many2One(Field): ``SET NULL`` will be changed into ``RESTRICT`` if required is set. :param datetime_field: The name of the field that contains the datetime value to read the target record. + :param target_search: The kind of target search 'subquery' or 'join' ''' self.__required = required if ondelete not in ('CASCADE', 'RESTRICT', 'SET NULL'): @@ -51,6 +52,8 @@ class Many2One(Field): self.left = left self.right = right self.datetime_field = datetime_field + assert target_search in ['subquery', 'join'] + self.target_search = target_search __init__.__doc__ += Field.__init__.__doc__ def __get_required(self): @@ -132,7 +135,10 @@ class Many2One(Field): return expression def convert_domain(self, domain, tables, Model): + pool = Pool() + Rule = pool.get('ir.rule') Target = self.get_target() + table, _ = tables[None] name, operator, value = domain[:3] column = self.sql_column(table) @@ -176,8 +182,18 @@ class Many2One(Field): target_domain = [(target_name,) + tuple(domain[1:])] if 'active' in Target._fields: target_domain.append(('active', 'in', [True, False])) - query = Target.search(target_domain, order=[], query=True) - return column.in_(query) + if self.target_search == 'subquery': + query = Target.search(target_domain, order=[], query=True) + return column.in_(query) + else: + target_tables = self._get_target_tables(tables) + target_table, _ = target_tables[None] + _, expression = Target.search_domain( + target_domain, tables=target_tables) + rule_domain = Rule.domain_get(Target.__name__, mode='read') + if rule_domain: + expression &= target_table.id.in_(rule_domain) + return expression def convert_order(self, name, tables, Model): fname, _, oexpr = name.partition('.') @@ -202,11 +218,17 @@ class Many2One(Field): return [self.sql_column(table)] ofield = Target._fields[oname] - target_tables = tables.get(fname) + target_tables = self._get_target_tables(tables) + return ofield.convert_order(oexpr, target_tables, Target) + + def _get_target_tables(self, tables): + Target = self.get_target() + table, _ = tables[None] + target_tables = tables.get(self.name) if target_tables is None: target = Target.__table__() target_tables = { None: (target, target.id == self.sql_column(table)), } - tables[fname] = target_tables - return ofield.convert_order(oexpr, target_tables, Target) + tables[self.name] = target_tables + return target_tables diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index 71acbfe883..29c567a17f 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -1140,7 +1140,7 @@ class ModelSQL(ModelStorage): return cls.browse([x['id'] for x in rows]) @classmethod - def search_domain(cls, domain, active_test=True): + def search_domain(cls, domain, active_test=True, tables=None): ''' Return SQL tables and expression Set active_test to add it. @@ -1148,9 +1148,10 @@ class ModelSQL(ModelStorage): transaction = Transaction() domain = cls._search_domain_active(domain, active_test=active_test) - tables = { - None: (cls.__table__(), None) - } + if tables is None: + tables = {} + if None not in tables: + tables[None] = (cls.__table__(), None) if cls._history and transaction.context.get('_datetime'): tables[None] = (cls.__table_history__(), None) diff --git a/trytond/tests/__init__.py b/trytond/tests/__init__.py index 9482db7057..f841378e9d 100644 --- a/trytond/tests/__init__.py +++ b/trytond/tests/__init__.py @@ -159,6 +159,7 @@ def register(): Many2OneTarget, Many2OneDomainValidation, Many2OneOrderBy, + Many2OneSearch, TestHistory, TestHistoryLine, FieldContextChild, diff --git a/trytond/tests/test.py b/trytond/tests/test.py index 8b4b201428..d6377dd56a 100644 --- a/trytond/tests/test.py +++ b/trytond/tests/test.py @@ -36,6 +36,7 @@ __all__ = [ 'DictSchema', 'Dict', 'DictDefault', 'DictRequired', 'Binary', 'BinaryDefault', 'BinaryRequired', 'Many2OneDomainValidation', 'Many2OneTarget', 'Many2OneOrderBy', + 'Many2OneSearch', ] @@ -734,3 +735,9 @@ class Many2OneOrderBy(ModelSQL): "Many2One OrderBy" __name__ = 'test.many2one_orderby' many2one = fields.Many2One('test.many2one_target', 'many2one') + + +class Many2OneSearch(ModelSQL): + "Many2One Search" + __name__ = 'test.many2one_search' + many2one = fields.Many2One('test.many2one_target', 'many2one') diff --git a/trytond/tests/test_fields.py b/trytond/tests/test_fields.py index bda99baecd..c6c64cf190 100644 --- a/trytond/tests/test_fields.py +++ b/trytond/tests/test_fields.py @@ -118,6 +118,7 @@ class FieldsTestCase(unittest.TestCase): self.m2o_domain_validation = POOL.get('test.many2one_domainvalidation') self.m2o_orderby = POOL.get('test.many2one_orderby') self.m2o_target = POOL.get('test.many2one_target') + self.m2o_search = POOL.get('test.many2one_search') def test0010boolean(self): 'Test Boolean' @@ -3265,6 +3266,30 @@ class FieldsTestCase(unittest.TestCase): transaction.cursor.rollback() + with Transaction().start(DB_NAME, USER, context=CONTEXT): + + target1, target2 = self.m2o_target.create([ + {'value': 1}, + {'value': 2}, + ]) + + search1, search2 = self.m2o_search.create([ + {'many2one': target1.id}, + {'many2one': target2.id}, + ]) + + # Search join + self.m2o_search.target_search = 'join' + self.assertEqual(self.m2o_search.search([ + ('many2one.value', '=', 1), + ]), [search1]) + + # Search subquery + self.m2o_search.target_search = 'subquery' + self.assertEqual(self.m2o_search.search([ + ('many2one.value', '=', 1), + ]), [search1]) + def test0200timedelta(self): 'Test timedelta' with Transaction().start(DB_NAME, USER,