Ensure to join with only one translation row

If there are multiple translations for the same record, the join will generate
result with multiple times the same id on ModelSQL.search.
As DISTINCT ON is not always supported, we must also ensure that duplicated
translations are all updated.

issue10061
review351261002

trytond/ir/translation.py

@@ -404,32 +404,34 @@ class Translation(ModelSQL, ModelView):
                     return record.model + ',' + field_name
 
             with Transaction().set_context(_check_access=False):
-                translations = {}
+                name2translations = defaultdict(list)
                 for translation in cls.search([
                             ('lang', '=', lang),
                             ('type', '=', ttype),
                             ('name', 'in', [get_name(r) for r in records]),
                             ]):
-                    translations[translation.name] = translation
+                    name2translations[translation.name].append(translation)
 
                 to_save = []
                 for record, value in zip(records, values):
-                    translation = translations.get(get_name(record))
+                    translations = name2translations.get(get_name(record))
                     if lang == 'en':
                         src = value
                     else:
                         src = getattr(record, field_name)
-                    if not translation:
+                    if not translations:
                         if not src and not value:
                             continue
                         translation = cls()
                         translation.name = name
                         translation.lang = lang
                         translation.type = ttype
-                    translation.src = src
-                    translation.value = value
-                    translation.fuzzy = False
-                    to_save.append(translation)
+                        translations.append(translation)
+                    for translation in translations:
+                        translation.src = src
+                        translation.value = value
+                        translation.fuzzy = False
+                        to_save.append(translation)
                 cls.save(to_save)
             return
 
@@ -437,7 +439,8 @@ class Translation(ModelSQL, ModelView):
         with Transaction().set_context(language=Config.get_language()):
             records = Model.browse(ids)
 
-        translations = {}
+        id2translations = defaultdict(list)
+        other_translations = defaultdict(list)
         with Transaction().set_context(_check_access=False):
             for translation in cls.search([
                         ('lang', '=', lang),
@@ -445,9 +448,8 @@ class Translation(ModelSQL, ModelView):
                         ('name', '=', name),
                         ('res_id', 'in', ids),
                         ]):
-                translations[translation.res_id] = translation
+                id2translations[translation.res_id].append(translation)
 
-            other_translations = {}
             if (lang == Config.get_language()
                     and Transaction().context.get('fuzzy_translation', True)):
                 for translation in cls.search([
@@ -456,17 +458,16 @@ class Translation(ModelSQL, ModelView):
                             ('name', '=', name),
                             ('res_id', 'in', ids),
                             ]):
-                    other_translations.setdefault(translation.res_id, []
-                        ).append(translation)
+                    other_translations[translation.res_id].append(translation)
 
             to_save = []
             for record, value in zip(records, values):
-                translation = translations.get(record.id)
+                translations = id2translations[record.id]
                 if lang == Config.get_language():
                     src = value
                 else:
                     src = getattr(record, field_name)
-                if not translation:
+                if not translations:
                     if not src and not value:
                         continue
                     translation = cls()
@@ -474,17 +475,19 @@ class Translation(ModelSQL, ModelView):
                     translation.lang = lang
                     translation.type = ttype
                     translation.res_id = record.id
+                    translations.append(translation)
                 else:
-                    other_langs = other_translations.get(record.id)
+                    other_langs = other_translations[record.id]
                     if other_langs:
                         for other_lang in other_langs:
                             other_lang.src = src
                             other_lang.fuzzy = True
                             to_save.append(other_lang)
-                translation.value = value
-                translation.src = src
-                translation.fuzzy = False
-                to_save.append(translation)
+                for translation in translations:
+                    translation.value = value
+                    translation.src = src
+                    translation.fuzzy = False
+                    to_save.append(translation)
             cls.save(to_save)
 
     @classmethod

trytond/model/fields/field.py

@@ -3,11 +3,14 @@
 import warnings
 from functools import wraps
 
+import sql
 from sql import (operators, Column, Literal, Select, CombiningQuery, Null,
     Query, Expression, Cast)
+from sql.aggregate import Min
 from sql.conditionals import Coalesce, NullIf
 from sql.operators import Concat
 
+from trytond import backend
 from trytond.cache import LRUDictTransaction
 from trytond.const import OPERATORS
 from trytond.pool import Pool
@@ -16,6 +19,8 @@ from trytond.rpc import RPC
 from trytond.tools.string_ import StringPartitioned, LazyString
 from trytond.transaction import Transaction
 
+_sql_version = tuple(map(int, sql.__version__.split('.')))
+
 
 def domain_validate(value):
     assert isinstance(value, list), 'domain must be a list'
@@ -492,34 +497,40 @@ class FieldTranslate(Field):
     def _get_translation_join(self, Model, name,
             translation, model, table, from_, language):
         if Model.__name__ == 'ir.model':
-            return from_.join(translation, 'LEFT',
-                condition=(translation.name == Concat(Concat(
-                            table.model, ','), name))
-                & (translation.res_id == -1)
-                & (translation.lang == language)
-                & (translation.type == 'model')
-                & (translation.fuzzy == Literal(False)))
+            name_ = Concat(Concat(table.model, ','), name)
+            type_ = 'model'
+            res_id = -1
         elif Model.__name__ == 'ir.model.field':
+            name_ = Concat(Concat(model.model, ','), table.name)
             if name == 'field_description':
                 type_ = 'field'
             else:
                 type_ = 'help'
-            return from_.join(model, 'LEFT',
-                condition=model.id == table.model).join(
-                    translation, 'LEFT',
-                    condition=(translation.name == Concat(Concat(
-                                model.model, ','), table.name))
-                    & (translation.res_id == -1)
-                    & (translation.lang == language)
-                    & (translation.type == type_)
-                    & (translation.fuzzy == Literal(False)))
+            res_id = -1
         else:
-            return from_.join(translation, 'LEFT',
-                condition=(translation.res_id == table.id)
-                & (translation.name == '%s,%s' % (Model.__name__, name))
-                & (translation.lang == language)
-                & (translation.type == 'model')
-                & (translation.fuzzy == Literal(False)))
+            name_ = '%s,%s' % (Model.__name__, name)
+            type_ = 'model'
+            res_id = table.id
+        if backend.name == 'postgresql' and _sql_version >= (1, 1, 0):
+            query = translation.select(
+                translation.res_id.as_('res_id'),
+                translation.value.as_('value'),
+                distinct=True,
+                distinct_on=[translation.res_id],
+                order_by=[translation.res_id, translation.id.desc])
+        else:
+            query = translation.select(
+                translation.res_id.as_('res_id'),
+                Min(translation.value).as_('value'),
+                group_by=[translation.res_id])
+        query.where = (
+            (translation.lang == language)
+            & (translation.type == type_)
+            & (translation.name == name_)
+            & (translation.fuzzy == Literal(False))
+            )
+        return query, from_.join(query, 'LEFT',
+            condition=(query.res_id == res_id))
 
     def convert_domain(self, domain, tables, Model):
         from trytond.ir.lang import get_parent_language
@@ -537,7 +548,7 @@ class FieldTranslate(Field):
         column = None
         while language:
             translation = Translation.__table__()
-            join = self._get_translation_join(
+            translation, join = self._get_translation_join(
                 Model, name, translation, model, table, join, language)
             column = Coalesce(NullIf(column, ''), translation.value)
             language = get_parent_language(language)
@@ -573,7 +584,7 @@ class FieldTranslate(Field):
             if key not in tables:
                 translation = Translation.__table__()
                 model = IrModel.__table__()
-                join = self._get_translation_join(
+                translation, join = self._get_translation_join(
                     Model, name, translation, model, table, table, language)
                 if join.left == table:
                     tables[key] = {

trytond/tests/test_modelsql.py

@@ -776,6 +776,64 @@ class ModelSQLTranslationTestCase(TranslationTestCase):
         self.assertEqual(record.name, "Foo")
         self.assertEqual(other.name, "Baz")
 
+    @with_transaction()
+    def test_search_unique_result(self):
+        "Test unique result on search"
+        pool = Pool()
+        Model = pool.get('test.modelsql.translation')
+        Translation = pool.get('ir.translation')
+
+        with Transaction().set_context(language=self.default_language):
+            record, = Model.create([{'name': "Foo"}])
+        with Transaction().set_context(language=self.other_language):
+            Model.write([record], {'name': "Bar"})
+
+        translation, = Translation.search([
+                ('lang', '=', self.other_language),
+                ('name', '=', 'test.modelsql.translation,name'),
+                ('type', '=', 'model'),
+                ('res_id', '=', record.id),
+                ])
+        Translation.copy([translation], default={'value': "Baz"})
+
+        with Transaction().set_context(language=self.other_language):
+            self.assertEqual(
+                Model.search([('name', 'like', 'Ba%')]),
+                [record])
+            self.assertEqual(
+                Model.search([], order=[('name', 'DESC')]),
+                [record])
+
+    @unittest.skipIf(backend.name != 'postgresql',
+        "Only PostgreSQL support DISTINCT ON")
+    @with_transaction()
+    def test_search_last_translation(self):
+        "Test unique result on search"
+        pool = Pool()
+        Model = pool.get('test.modelsql.translation')
+        Translation = pool.get('ir.translation')
+
+        with Transaction().set_context(language=self.default_language):
+            record, = Model.create([{'name': "Foo"}])
+        with Transaction().set_context(language=self.other_language):
+            Model.write([record], {'name': "Bar"})
+
+        translation, = Translation.search([
+                ('lang', '=', self.other_language),
+                ('name', '=', 'test.modelsql.translation,name'),
+                ('type', '=', 'model'),
+                ('res_id', '=', record.id),
+                ])
+        Translation.copy([translation], default={'value': "Baz"})
+
+        with Transaction().set_context(language=self.other_language):
+            self.assertEqual(
+                Model.search([('name', '=', 'Baz')]),
+                [record])
+            self.assertEqual(
+                Model.search([('name', '=', 'Bar')]),
+                [])
+
     @with_transaction()
     def test_search_fill_transaction_cache(self):
         "Test search fill the transaction cache"