[IMP] core, base: allow disabling unaccent on a per-field basis
For searches, Odoo uses `unaccent` if it's available. On some technical fields this is completely unnecessary and precludes the use of indexes. This PR provides: * an opt-out (`unaccent = False`) on `String` and `Text` fields * a warning if `unaccent` is enabled on *parent_path* fields as their performance can be rather critical and not using the index is quite an issue (note: the check that `parent_path` fields have been moved outside of the check for their existence as we want to check that the field is declared and correctly configured in all cases, probably) Task 2627454 Part-of: odoo/odoo#76436
This commit is contained in:
@@ -1,5 +1,7 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
import psycopg2
|
||||
|
||||
@@ -672,13 +674,29 @@ class TestExpression(SavepointCaseWithUserDemo):
|
||||
|
||||
def test_accent(self):
|
||||
if not self.registry.has_unaccent:
|
||||
return
|
||||
Company = self.env['res.company']
|
||||
helene = Company.create({'name': u'Hélène'})
|
||||
self.assertEqual(helene, Company.search([('name','ilike','Helene')]))
|
||||
self.assertEqual(helene, Company.search([('name','ilike','hélène')]))
|
||||
self.assertNotIn(helene, Company.search([('name','not ilike','Helene')]))
|
||||
self.assertNotIn(helene, Company.search([('name','not ilike','hélène')]))
|
||||
raise unittest.SkipTest("unaccent not enabled")
|
||||
|
||||
Model = self.env['res.partner.category']
|
||||
helen = Model.create({'name': 'Hélène'})
|
||||
self.assertEqual(helen, Model.search([('name', 'ilike', 'Helene')]))
|
||||
self.assertEqual(helen, Model.search([('name', 'ilike', 'hélène')]))
|
||||
self.assertNotIn(helen, Model.search([('name', 'not ilike', 'Helene')]))
|
||||
self.assertNotIn(helen, Model.search([('name', 'not ilike', 'hélène')]))
|
||||
|
||||
hermione, nicostratus = Model.create([
|
||||
{'name': 'Hermione', 'parent_id': helen.id},
|
||||
{'name': 'Nicostratus', 'parent_id': helen.id}
|
||||
])
|
||||
self.assertEqual(nicostratus.parent_path, f'{helen.id}/{nicostratus.id}/')
|
||||
|
||||
with patch('odoo.osv.expression.get_unaccent_wrapper') as w:
|
||||
w().side_effect = lambda x: x
|
||||
rs = Model.search([('parent_path', 'like', f'{helen.id}/%')], order='id asc')
|
||||
self.assertEqual(rs, helen | hermione | nicostratus)
|
||||
# the result of `get_unaccent_wrapper()` is the wrapper and that's
|
||||
# what should not be called
|
||||
w().assert_not_called()
|
||||
|
||||
|
||||
def test_pure_function(self):
|
||||
orig_false = expression.FALSE_DOMAIN.copy()
|
||||
|
||||
@@ -1536,6 +1536,7 @@ class _String(Field):
|
||||
""" Abstract class for string fields. """
|
||||
translate = False # whether the field is translated
|
||||
prefetch = None
|
||||
unaccent = True
|
||||
|
||||
def __init__(self, string=Default, **kwargs):
|
||||
# translate is either True, False, or a callable
|
||||
|
||||
+10
-7
@@ -2817,8 +2817,9 @@ class BaseModel(metaclass=MetaModel):
|
||||
|
||||
if self._parent_store:
|
||||
if not tools.column_exists(cr, self._table, 'parent_path'):
|
||||
self._create_parent_columns()
|
||||
tools.create_column(self._cr, self._table, 'parent_path', 'VARCHAR')
|
||||
parent_path_compute = True
|
||||
self._check_parent_path()
|
||||
|
||||
if not must_create_table:
|
||||
self._check_removed_columns(log=False)
|
||||
@@ -2861,12 +2862,14 @@ class BaseModel(metaclass=MetaModel):
|
||||
"""
|
||||
pass
|
||||
|
||||
def _create_parent_columns(self):
|
||||
tools.create_column(self._cr, self._table, 'parent_path', 'VARCHAR')
|
||||
if 'parent_path' not in self._fields:
|
||||
_logger.error("add a field parent_path on model %s: parent_path = fields.Char(index=True)", self._name)
|
||||
elif not self._fields['parent_path'].index:
|
||||
_logger.error('parent_path field on model %s must be indexed! Add index=True to the field definition)', self._name)
|
||||
def _check_parent_path(self):
|
||||
field = self._fields.get('parent_path')
|
||||
if field is None:
|
||||
_logger.error("add a field parent_path on model %r: `parent_path = fields.Char(index=True, unaccent=False)`.", self._name)
|
||||
elif not field.index:
|
||||
_logger.error('parent_path field on model %r should be indexed! Add index=True to the field definition.', self._name)
|
||||
elif field.unaccent:
|
||||
_logger.warning("parent_path field on model %r should have unaccent disabled. Add `unaccent=False` to the field definition.", self._name)
|
||||
|
||||
def _add_sql_constraints(self):
|
||||
"""
|
||||
|
||||
+15
-9
@@ -436,7 +436,7 @@ class expression(object):
|
||||
:attr result: the result of the parsing, as a pair (query, params)
|
||||
:attr query: Query object holding the final result
|
||||
"""
|
||||
self._unaccent = get_unaccent_wrapper(model._cr)
|
||||
self._unaccent_wrapper = get_unaccent_wrapper(model._cr)
|
||||
self.root_model = model
|
||||
self.root_alias = alias or model._table
|
||||
|
||||
@@ -449,6 +449,11 @@ class expression(object):
|
||||
# parse the domain expression
|
||||
self.parse()
|
||||
|
||||
def _unaccent(self, field, _id=lambda x: x):
|
||||
if getattr(field, 'unaccent', False):
|
||||
return self._unaccent_wrapper
|
||||
return _id
|
||||
|
||||
# ----------------------------------------
|
||||
# Leafs management
|
||||
# ----------------------------------------
|
||||
@@ -930,7 +935,7 @@ class expression(object):
|
||||
if sql_operator in ('in', 'not in'):
|
||||
right = tuple(right)
|
||||
|
||||
unaccent = self._unaccent if sql_operator.endswith('like') else lambda x: x
|
||||
unaccent = self._unaccent(field) if sql_operator.endswith('like') else lambda x: x
|
||||
|
||||
left = unaccent(model._generate_translated_field(alias, left, self.query))
|
||||
instr = unaccent('%s')
|
||||
@@ -1043,16 +1048,18 @@ class expression(object):
|
||||
query, params = self.__leaf_to_sql((left, '=', right), model, alias)
|
||||
|
||||
else:
|
||||
field = model._fields.get(left)
|
||||
if field is None:
|
||||
raise ValueError("Invalid field %r in domain term %r" % (left, leaf))
|
||||
|
||||
need_wildcard = operator in ('like', 'ilike', 'not like', 'not ilike')
|
||||
sql_operator = {'=like': 'like', '=ilike': 'ilike'}.get(operator, operator)
|
||||
cast = '::text' if sql_operator.endswith('like') else ''
|
||||
cast = '::text' if sql_operator.endswith('like') else ''
|
||||
|
||||
if left not in model:
|
||||
raise ValueError("Invalid field %r in domain term %r" % (left, leaf))
|
||||
format = '%s' if need_wildcard else model._fields[left].column_format
|
||||
unaccent = self._unaccent if sql_operator.endswith('like') else lambda x: x
|
||||
column_format = '%s' if need_wildcard else field.column_format
|
||||
unaccent = self._unaccent(field) if sql_operator.endswith('like') else lambda x: x
|
||||
column = '%s.%s' % (table_alias, _quote(left))
|
||||
query = '(%s %s %s)' % (unaccent(column + cast), sql_operator, unaccent(format))
|
||||
query = '(%s %s %s)' % (unaccent(column + cast), sql_operator, unaccent(column_format))
|
||||
|
||||
if (need_wildcard and not right) or (right and operator in NEGATIVE_TERM_OPERATORS):
|
||||
query = '(%s OR %s."%s" IS NULL)' % (query, table_alias, left)
|
||||
@@ -1060,7 +1067,6 @@ class expression(object):
|
||||
if need_wildcard:
|
||||
params = ['%%%s%%' % pycompat.to_text(right)]
|
||||
else:
|
||||
field = model._fields[left]
|
||||
params = [field.convert_to_column(right, model, validate=False)]
|
||||
|
||||
return query, params
|
||||
|
||||
Reference in New Issue
Block a user