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
