changeset 428bf1c84a3d in trytond:default details: https://hg.tryton.org/trytond?cmd=changeset&node=428bf1c84a3d description: Include order columns in UNION-ed search queries
issue10790 review365991002 diffstat: trytond/model/modelsql.py | 59 ++++++++++++-- trytond/tests/modelsql.py | 17 ++++ trytond/tests/test_modelsql.py | 167 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 233 insertions(+), 10 deletions(-) diffs (324 lines): diff -r 9ca2feb6cc84 -r 428bf1c84a3d trytond/model/modelsql.py --- a/trytond/model/modelsql.py Mon Oct 04 18:52:01 2021 +0200 +++ b/trytond/model/modelsql.py Wed Oct 06 17:20:40 2021 +0200 @@ -1282,7 +1282,7 @@ raise AccessError(msg) @classmethod - def __search_query(cls, domain, count, query): + def __search_query(cls, domain, count, query, order): pool = Pool() Rule = pool.get('ir.rule') @@ -1296,6 +1296,34 @@ local_domains.insert(0, 'OR') joined_domains.append(local_domains) + def get_local_columns(order_exprs): + local_columns = [] + for order_expr in order_exprs: + if (isinstance(order_expr, Column) + and isinstance(order_expr._from, Table) + and order_expr._from._name == cls._table): + local_columns.append(order_expr._name) + else: + raise NotImplementedError + return local_columns + + # The UNION optimization needs the columns used to order the query + extra_columns = set() + if order and joined_domains: + tables = { + None: (cls.__table__(), None), + } + for oexpr, otype in order: + fname = oexpr.partition('.')[0] + field = cls._fields[fname] + field_orders = field.convert_order(oexpr, tables, cls) + try: + order_columns = get_local_columns(field_orders) + extra_columns.update(order_columns) + except NotImplementedError: + joined_domains = None + break + # In case the search uses subqueries it's more efficient to use a UNION # of queries than using clauses with some JOIN because databases can # used indexes @@ -1309,8 +1337,9 @@ expression &= domain_exp main_table, _ = tables[None] table = convert_from(None, tables) - columns = cls.__searched_columns( - main_table, not count and not query) + columns = cls.__searched_columns(main_table, + eager=not count and not query, + extra_columns=extra_columns) union_tables.append(table.select( *columns, where=expression)) expression = None @@ -1327,20 +1356,30 @@ return tables, expression @classmethod - def __searched_columns(cls, table, eager=False, history=False): + def __searched_columns( + cls, table, *, eager=False, history=False, extra_columns=None): + if extra_columns is None: + extra_columns = [] + else: + extra_columns = sorted(extra_columns - {'id', '__id', '_datetime'}) columns = [table.id.as_('id')] if (cls._history and Transaction().context.get('_datetime') and (eager or history)): columns.append( Coalesce(table.write_date, table.create_date).as_('_datetime')) columns.append(Column(table, '__id').as_('__id')) + for column_name in extra_columns: + field = cls._fields[column_name] + sql_column = field.sql_column(table).as_(column_name) + columns.append(sql_column) if eager: columns += [f.sql_column(table).as_(n) - for n, f in cls._fields.items() + for n, f in sorted(cls._fields.items()) if not hasattr(f, 'get') - and n != 'id' - and not getattr(f, 'translate', False) - and f.loading == 'eager'] + and n not in extra_columns + and n != 'id' + and not getattr(f, 'translate', False) + and f.loading == 'eager'] if not callable(cls.table_query): sql_type = fields.Char('timestamp').sql_type().base columns += [Extract('EPOCH', @@ -1389,7 +1428,7 @@ super(ModelSQL, cls).search( domain, offset=offset, limit=limit, order=order, count=count) - tables, expression = cls.__search_query(domain, count, query) + tables, expression = cls.__search_query(domain, count, query, order) main_table, _ = tables[None] if count: @@ -1401,7 +1440,7 @@ order_by = cls.__search_order(order, tables) # compute it here because __search_order might modify tables table = convert_from(None, tables) - columns = cls.__searched_columns(main_table, not query) + columns = cls.__searched_columns(main_table, eager=not query) select = table.select( *columns, where=expression, limit=limit, offset=offset, order_by=order_by) diff -r 9ca2feb6cc84 -r 428bf1c84a3d trytond/tests/modelsql.py --- a/trytond/tests/modelsql.py Mon Oct 04 18:52:01 2021 +0200 +++ b/trytond/tests/modelsql.py Wed Oct 06 17:20:40 2021 +0200 @@ -83,6 +83,22 @@ name = fields.Char("Name") +class ModelSQLSearchOR2Union(ModelSQL): + "ModelSQL Search OR to UNION optimization" + __name__ = 'test.modelsql.search.or2union' + name = fields.Char("Name") + target = fields.Many2One('test.modelsql.read.target', "Target") + targets = fields.One2Many('test.modelsql.read.target', 'parent', "Targets") + reference = fields.Reference( + "Reference", [(None, ""), ('test.modelsql.read.target', "Target")]) + integer = fields.Integer("Integer") + + @classmethod + def order_integer(cls, tables): + table, _ = tables[None] + return [table.integer + 1] + + class ModelSQLForeignKey(DeactivableMixin, ModelSQL): "ModelSQL Foreign Key" __name__ = 'test.modelsql.fk' @@ -178,6 +194,7 @@ ModelSQLOne2Many, ModelSQLOne2ManyTarget, ModelSQLSearch, + ModelSQLSearchOR2Union, ModelSQLForeignKey, ModelSQLForeignKeyTarget, NullOrder, diff -r 9ca2feb6cc84 -r 428bf1c84a3d trytond/tests/test_modelsql.py --- a/trytond/tests/test_modelsql.py Mon Oct 04 18:52:01 2021 +0200 +++ b/trytond/tests/test_modelsql.py Wed Oct 06 17:20:40 2021 +0200 @@ -678,6 +678,173 @@ result_without_split) @with_transaction() + def test_search_or_to_union_order_eager_field(self): + """ + Searching for 'OR'-ed domain mixed with ordering on an eager field + """ + pool = Pool() + Model = pool.get('test.modelsql.search.or2union') + Target = pool.get('test.modelsql.read.target') + + target_a, target_b, target_c = Target.create([ + {'name': 'A'}, {'name': 'B'}, {'name': 'C'}, + ]) + model_a, model_b, model_c = Model.create([{ + 'name': 'A', + 'target': target_a, + }, { + 'name': 'B', + 'target': target_b, + }, { + 'name': 'C', + 'target': target_c, + 'targets': [('create', [{ + 'name': 'C.A', + }]), + ], + }]) + + domain = ['OR', + ('name', 'ilike', '%A%'), + ('targets.name', 'ilike', '%A'), + ] + self.assertEqual( + Model.search(domain, order=[('name', 'ASC')]), + [model_a, model_c]) + self.assertEqual( + Model.search(domain, order=[('name', 'DESC')]), + [model_c, model_a]) + self.assertIn( + 'UNION', + str(Model.search(domain, order=[('name', 'ASC')], query=True))) + + @with_transaction() + def test_search_or_to_union_order_lazy_field(self): + """ + Searching for 'OR'-ed domain mixed with ordering on a lazy field + """ + pool = Pool() + Model = pool.get('test.modelsql.search.or2union') + Target = pool.get('test.modelsql.read.target') + + target_a, target_b, target_c = Target.create([ + {'name': 'A'}, {'name': 'B'}, {'name': 'C'}, + ]) + model_a, model_b, model_c = Model.create([{ + 'name': 'A', + 'reference': str(target_a), + }, { + 'name': 'B', + 'reference': str(target_b), + }, { + 'name': 'C', + 'reference': str(target_c), + 'targets': [('create', [{ + 'name': 'C.A', + }]), + ], + }]) + + domain = ['OR', + ('name', 'ilike', '%A%'), + ('targets.name', 'ilike', '%A'), + ] + self.assertEqual( + Model.search(domain, order=[('reference', 'ASC')]), + [model_a, model_c]) + self.assertEqual( + Model.search(domain, order=[('reference', 'DESC')]), + [model_c, model_a]) + self.assertIn( + 'UNION', str(Model.search( + domain, order=[('reference', 'ASC')], query=True))) + + @with_transaction() + def test_search_or_to_union_order_dotted_notation(self): + """ + Searching for 'OR'-ed domain mixed with ordering on dotted field + """ + pool = Pool() + Model = pool.get('test.modelsql.search.or2union') + Target = pool.get('test.modelsql.read.target') + + target_a, target_b, target_c = Target.create([ + {'name': 'A'}, {'name': 'B'}, {'name': 'C'}, + ]) + model_a, model_b, model_c = Model.create([{ + 'name': 'A', + 'target': target_a, + }, { + 'name': 'B', + 'target': target_b, + }, { + 'name': 'C', + 'target': target_c, + 'targets': [('create', [{ + 'name': 'C.A', + }]), + ], + }]) + + domain = ['OR', + ('name', 'ilike', '%A%'), + ('targets.name', 'ilike', '%A'), + ] + self.assertEqual( + Model.search(domain, order=[('target.name', 'ASC')]), + [model_a, model_c]) + self.assertEqual( + Model.search(domain, order=[('target.name', 'DESC')]), + [model_c, model_a]) + self.assertNotIn( + 'UNION', str(Model.search( + domain, order=[('target.name', 'ASC')], query=True))) + + @with_transaction() + def test_search_or_to_union_order_function(self): + """ + Searching for 'OR'-ed domain mixed with ordering on a function + """ + pool = Pool() + Model = pool.get('test.modelsql.search.or2union') + Target = pool.get('test.modelsql.read.target') + + target_a, target_b, target_c = Target.create([ + {'name': 'A'}, {'name': 'B'}, {'name': 'C'}, + ]) + model_a, model_b, model_c = Model.create([{ + 'name': 'A', + 'target': target_a, + 'integer': 0, + }, { + 'name': 'B', + 'target': target_b, + 'integer': 1, + }, { + 'name': 'C', + 'target': target_c, + 'integer': 2, + 'targets': [('create', [{ + 'name': 'C.A', + }]), + ], + }]) + + domain = ['OR', + ('name', 'ilike', '%A%'), + ('targets.name', 'ilike', '%A'), + ] + self.assertEqual( + Model.search(domain, order=[('integer', 'ASC')]), + [model_a, model_c]) + self.assertEqual( + Model.search(domain, order=[('integer', 'DESC')]), + [model_c, model_a]) + self.assertNotIn( + 'UNION', str(Model.search( + domain, order=[('integer', 'ASC')], query=True))) + + @with_transaction() def test_search_or_to_union_no_local_clauses(self): """ Test searching for 'OR'-ed domain without local clauses