[FIX] core: make index naming without conflicts

There are two issues with the index naming convention used by the ORM:

Problem 1: it is possible to have naming conflict for indexes.  For
instance, the name 'slide_channel_tag_group_sequence_index' is used for
both fields slide.channel.tag.group.sequence and
slide.channel.tag.group_sequence.  Only the first index will be created.

Solution 1: we separate the model and field names with a double
underscore instead of a single one, which is the same strategy as with
LEFT JOIN aliases.  This is correct because model names don't contain
such double underscores or underscores as prefix or suffix (it is not
forbiden but model name should follow the 'dot notation'.)

Problem 2: index names can be longer than 63 chars, but PostgreSQL
silently truncates it.  This doesn't actually break anything (PostgreSQL
also truncates values when we check the existence of indexes) but it can
lead to using the same name twice.  There is hopefully not any example
in our code.

Solution 2: if the name is too large, we truncate it and pad it with a
hash of the complete name to match 63 characters, which is also the
strategy used for LEFT JOIN aliases.

task-2984730

closes odoo/odoo#100736

Related: odoo/upgrade#3957
Signed-off-by: Raphael Collet <rco@odoo.com>

odoo/addons/test_new_api/models/test_new_api.py

@@ -530,6 +530,7 @@ class Order(models.Model):
     _name = _description = 'test_new_api.order'
 
     line_ids = fields.One2many('test_new_api.order.line', 'order_id')
+    line_short_field_name = fields.Integer(index=True)
 
 
 class OrderLine(models.Model):
@@ -538,6 +539,9 @@ class OrderLine(models.Model):
     order_id = fields.Many2one('test_new_api.order', required=True, ondelete='cascade')
     product = fields.Char()
     reward = fields.Boolean()
+    short_field_name = fields.Integer(index=True)
+    very_very_very_very_very_long_field_name_1 = fields.Integer(index=True)
+    very_very_very_very_very_long_field_name_2 = fields.Integer(index=True)
 
     def unlink(self):
         # also delete associated reward lines

odoo/addons/test_new_api/tests/test_schema.py

@@ -577,3 +577,40 @@ class TestSchema(common.TransactionCase):
             (field.relation, field.column1, model._table, 'id', 'CASCADE'),
             (field.relation, field.column2, comodel._table, 'id', 'CASCADE'),
         ])
+
+    def test_20_unique_indexes(self):
+        """ Test uniqueness of indexes:
+        - test_new_api.order.line_short_field_name
+        - test_new_api.order.line.short_field_name
+        """
+        tablenames = ('test_new_api_order', 'test_new_api_order_line')
+        self.env.cr.execute("""
+            SELECT tablename
+            FROM pg_indexes
+            WHERE tablename IN %s AND indexdef LIKE %s
+        """, [tablenames, '%short_field_name%'])
+        tables = {table for table, in self.env.cr.fetchall()}
+        self.assertEqual(tables, {'test_new_api_order', 'test_new_api_order_line'})
+
+    def test_21_too_long_indexes(self):
+        """ Test too long indexes name:
+
+        Both indexes share same truncated name
+        'test_new_api_order_line__very_very_very_very_very_long_field_nam'
+        if no strategy is done to avoid duplicate too long index names
+
+        -  test_new_api.order.line.very_very_very_very_very_long_field_name_1
+        -> test_new_api_order_line__very_very_very_very_very_long_field_name_1_index
+        => test_new_api_order_line__very_very_very_very_very_long_ea4b39c9
+
+        -  test_new_api.order.line.very_very_very_very_very_long_field_name_2
+        -> test_new_api_order_line__very_very_very_very_very_long_field_name_2_index
+        => test_new_api_order_line__very_very_very_very_very_long_dba32354
+        """
+        self.env.cr.execute("""
+            SELECT COUNT(*)
+            FROM pg_indexes
+            WHERE tablename = 'test_new_api_order_line' AND indexdef LIKE %s
+        """, ['%very_very_very_very_long_field_name%'])
+        nb_field_index, = self.env.cr.fetchone()
+        self.assertEqual(nb_field_index, 2)

odoo/modules/registry.py

@@ -479,8 +479,9 @@ class Registry(Mapping):
 
     def check_indexes(self, cr, model_names):
         """ Create or drop column indexes for the given models. """
+
         expected = [
-            (f"{Model._table}_{field.name}_index", Model._table, field, getattr(field, 'unaccent', False))
+            (sql.make_index_name(Model._table, field.name), Model._table, field, getattr(field, 'unaccent', False))
             for model_name in model_names
             for Model in [self.models[model_name]]
             if Model._auto and not Model._abstract
@@ -496,12 +497,11 @@ class Registry(Mapping):
         existing = dict(cr.fetchall())
 
         for indexname, tablename, field, unaccent in expected:
-            column_expression = f'"{field.name}"'
             index = field.index
             assert index in ('btree', 'btree_not_null', 'trigram', True, False, None)
             if index and indexname not in existing and \
                     ((not field.translate and index != 'trigram') or (index == 'trigram' and self.has_trigram)):
-
+                column_expression = f'"{field.name}"'
                 if index == 'trigram':
                     if field.translate:
                         column_expression = f'''(jsonb_path_query_array({column_expression}, '$.*')::text)'''

odoo/tools/query.py

@@ -3,7 +3,8 @@
 
 import re
 import warnings
-from zlib import crc32
+
+from odoo.tools.sql import make_identifier
 
 from .func import lazy_property
 
@@ -37,14 +38,7 @@ def _generate_table_alias(src_table_alias, link):
         :param str link: field name
         :return str: alias
     """
-    alias = "%s__%s" % (src_table_alias, link)
-    # Use an alternate alias scheme if length exceeds the PostgreSQL limit
-    # of 63 characters.
-    if len(alias) >= 64:
-        # We have to fit a crc32 hash and one underscore into a 63 character
-        # alias. The remaining space we can use to add a human readable prefix.
-        alias = "%s_%08x" % (alias[:54], crc32(alias.encode('utf-8')))
-    return alias
+    return make_identifier(f"{src_table_alias}__{link}")
 
 
 class Query(object):

odoo/tools/sql.py

@@ -3,15 +3,14 @@
 
 # pylint: disable=sql-injection
 
+from binascii import crc32
 import logging
 import json
 import re
 import psycopg2
 from psycopg2.sql import SQL, Identifier
 
-import odoo.sql_db
 from collections import defaultdict
-from contextlib import closing
 
 _schema = logging.getLogger('odoo.schema')
 
@@ -421,3 +420,20 @@ def pattern_to_translated_trigram_pattern(pattern):
 
     # replace the original wildcard characters by %
     return f"%{'%'.join(wildcard_escaped)}%" if wildcard_escaped else "%"
+
+
+def make_identifier(identifier: str) -> str:
+    """ Return ``identifier``, possibly modified to fit PostgreSQL's identifier size limitation.
+    If too long, ``identifier`` is truncated and padded with a hash to make it mostly unique.
+    """
+    # if length exceeds the PostgreSQL limit of 63 characters.
+    if len(identifier) > 63:
+        # We have to fit a crc32 hash and one underscore into a 63 character
+        # alias. The remaining space we can use to add a human readable prefix.
+        return f"{identifier[:54]}_{crc32(identifier.encode()):08x}"
+    return identifier
+
+
+def make_index_name(table_name: str, column_name: str) -> str:
+    """ Return an index name according to conventions for the given table and column. """
+    return make_identifier(f"{table_name}__{column_name}_index")