Fixed #24442 -- Improved SchemaEditor's index name truncation.

4 files changed, 63 insertions, 42 deletions

diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py
index 39a7e5e623..67960a02ac 100644
--- a/django/db/backends/base/schema.py
+++ b/django/db/backends/base/schema.py
@@ -2,7 +2,6 @@ import hashlib
 import logging
 from datetime import datetime
 
-from django.db.backends.utils import truncate_name
 from django.db.transaction import atomic
 from django.utils import six, timezone
 from django.utils.encoding import force_bytes
@@ -831,32 +830,30 @@ class BaseDatabaseSchemaEditor(object):
     def _create_index_name(self, model, column_names, suffix=""):
         """
         Generates a unique name for an index/unique constraint.
+
+        The name is divided into 3 parts: the table name, the column names,
+        and a unique digest and suffix.
         """
-        # If there is just one column in the index, use a default algorithm from Django
-        if len(column_names) == 1 and not suffix:
-            return truncate_name(
-                '%s_%s' % (model._meta.db_table, self._digest(column_names[0])),
-                self.connection.ops.max_name_length()
-            )
-        # Else generate the name for the index using a different algorithm
-        table_name = model._meta.db_table.replace('"', '').replace('.', '_')
-        index_unique_name = '_%s' % self._digest(table_name, *column_names)
+        table_name = model._meta.db_table
+        hash_data = [table_name] + list(column_names)
+        hash_suffix_part = '%s%s' % (self._digest(*hash_data), suffix)
         max_length = self.connection.ops.max_name_length() or 200
-        # If the index name is too long, truncate it
-        index_name = ('%s_%s%s%s' % (
-            table_name, column_names[0], index_unique_name, suffix,
-        )).replace('"', '').replace('.', '_')
-        if len(index_name) > max_length:
-            part = ('_%s%s%s' % (column_names[0], index_unique_name, suffix))
-            index_name = '%s%s' % (table_name[:(max_length - len(part))], part)
-        # It shouldn't start with an underscore (Oracle hates this)
-        if index_name[0] == "_":
-            index_name = index_name[1:]
-        # If it's STILL too long, just hash it down
-        if len(index_name) > max_length:
-            index_name = hashlib.md5(force_bytes(index_name)).hexdigest()[:max_length]
-        # It can't start with a number on Oracle, so prepend D if we need to
-        if index_name[0].isdigit():
+        # If everything fits into max_length, use that name.
+        index_name = '%s_%s_%s' % (table_name, '_'.join(column_names), hash_suffix_part)
+        if len(index_name) <= max_length:
+            return index_name
+        # Shorten a long suffix.
+        if len(hash_suffix_part) > max_length / 3:
+            hash_suffix_part = hash_suffix_part[:max_length // 3]
+        other_length = (max_length - len(hash_suffix_part)) // 2 - 1
+        index_name = '%s_%s_%s' % (
+            table_name[:other_length],
+            '_'.join(column_names)[:other_length],
+            hash_suffix_part,
+        )
+        # Prepend D if needed to prevent the name from starting with an
+        # underscore or a number (not permitted on Oracle).
+        if index_name[0] == "_" or index_name[0].isdigit():
             index_name = "D%s" % index_name[:-1]
         return index_name
 
diff --git a/tests/indexes/tests.py b/tests/indexes/tests.py
index 4bd2332674..3ef6d2939b 100644
--- a/tests/indexes/tests.py
+++ b/tests/indexes/tests.py
@@ -18,10 +18,34 @@ class SchemaIndexesTests(TestCase):
         with connection.schema_editor() as editor:
             index_name = editor._create_index_name(
                 model=Article,
-                column_names=("c1", "c2", "c3"),
+                column_names=("c1",),
                 suffix="123",
             )
-        self.assertEqual(index_name, "indexes_article_c1_7ce4cc86123")
+        self.assertEqual(index_name, "indexes_article_c1_a52bd80b123")
+
+    def test_index_name(self):
+        """
+        Index names on the built-in database backends::
+            * Are truncated as needed.
+            * Include all the column names.
+            * Include a deterministic hash.
+        """
+        long_name = 'l%sng' % ('o' * 100)
+        with connection.schema_editor() as editor:
+            index_name = editor._create_index_name(
+                model=Article,
+                column_names=('c1', 'c2', long_name),
+                suffix='ix',
+            )
+        expected = {
+            'mysql': 'indexes_article_c1_c2_looooooooooooooooooo_255179b2ix',
+            'oracle': 'indexes_a_c1_c2_loo_255179b2ix',
+            'postgresql': 'indexes_article_c1_c2_loooooooooooooooooo_255179b2ix',
+            'sqlite': 'indexes_article_c1_c2_l%sng_255179b2ix' % ('o' * 100),
+        }
+        if connection.vendor not in expected:
+            self.skipTest('This test is only supported on the built-in database backends.')
+        self.assertEqual(index_name, expected[connection.vendor])
 
     def test_index_together(self):
         editor = connection.schema_editor()
@@ -71,6 +95,6 @@ class SchemaIndexesTests(TestCase):
             self.skip("This test only applies to the InnoDB storage engine")
         index_sql = connection.schema_editor()._model_indexes_sql(ArticleTranslation)
         self.assertEqual(index_sql, [
-            'CREATE INDEX `indexes_articletranslation_99fb53c2` '
+            'CREATE INDEX `indexes_articletranslation_article_no_constraint_id_d6c0806b` '
             'ON `indexes_articletranslation` (`article_no_constraint_id`)'
         ])
diff --git a/tests/postgres_tests/test_array.py b/tests/postgres_tests/test_array.py
index 6a305556d4..6dd1fb85e0 100644
--- a/tests/postgres_tests/test_array.py
+++ b/tests/postgres_tests/test_array.py
@@ -448,13 +448,13 @@ class TestMigrations(TransactionTestCase):
         table_name = 'postgres_tests_chartextarrayindexmodel'
         call_command('migrate', 'postgres_tests', verbosity=0)
         with connection.cursor() as cursor:
-            like_constraint_field_names = [
-                c.rsplit('_', 2)[0][len(table_name) + 1:]
-                for c in connection.introspection.get_constraints(cursor, table_name)
-                if c.endswith('_like')
+            like_constraint_columns_list = [
+                v['columns']
+                for k, v in list(connection.introspection.get_constraints(cursor, table_name).items())
+                if k.endswith('_like')
             ]
         # Only the CharField should have a LIKE index.
-        self.assertEqual(like_constraint_field_names, ['char2'])
+        self.assertEqual(like_constraint_columns_list, [['char2']])
         with connection.cursor() as cursor:
             indexes = connection.introspection.get_indexes(cursor, table_name)
         # All fields should have regular indexes.
diff --git a/tests/schema/tests.py b/tests/schema/tests.py
index 5b51bd36a9..6bb4aa6f8f 100644
--- a/tests/schema/tests.py
+++ b/tests/schema/tests.py
@@ -1917,7 +1917,7 @@ class SchemaTests(TransactionTestCase):
         # Should create two indexes; one for like operator.
         self.assertEqual(
             self.get_constraints_for_column(Author, 'nom_de_plume'),
-            ['schema_author_95aa9e9b', 'schema_author_nom_de_plume_7570a851_like'],
+            ['schema_author_nom_de_plume_7570a851', 'schema_author_nom_de_plume_7570a851_like'],
         )
 
     @unittest.skipUnless(connection.vendor == 'postgresql', "PostgreSQL specific")
@@ -1947,7 +1947,7 @@ class SchemaTests(TransactionTestCase):
             editor.alter_field(Author, old_field, new_field, strict=True)
         self.assertEqual(
             self.get_constraints_for_column(Author, 'name'),
-            ['schema_author_b068931c', 'schema_author_name_1fbc5617_like']
+            ['schema_author_name_1fbc5617', 'schema_author_name_1fbc5617_like']
         )
         # Remove db_index=True to drop both indexes.
         with connection.schema_editor() as editor:
@@ -1989,7 +1989,7 @@ class SchemaTests(TransactionTestCase):
             editor.alter_field(Note, old_field, new_field, strict=True)
         self.assertEqual(
             self.get_constraints_for_column(Note, 'info'),
-            ['schema_note_caf9b6b9', 'schema_note_info_4b0ea695_like']
+            ['schema_note_info_4b0ea695', 'schema_note_info_4b0ea695_like']
         )
         # Remove db_index=True to drop both indexes.
         with connection.schema_editor() as editor:
@@ -2003,7 +2003,7 @@ class SchemaTests(TransactionTestCase):
             editor.create_model(BookWithoutAuthor)
         self.assertEqual(
             self.get_constraints_for_column(BookWithoutAuthor, 'title'),
-            ['schema_book_d5d3db17', 'schema_book_title_2dfb2dff_like']
+            ['schema_book_title_2dfb2dff', 'schema_book_title_2dfb2dff_like']
         )
         # Alter to add unique=True (should replace the index)
         old_field = BookWithoutAuthor._meta.get_field('title')
@@ -2022,7 +2022,7 @@ class SchemaTests(TransactionTestCase):
             editor.alter_field(BookWithoutAuthor, new_field, new_field2, strict=True)
         self.assertEqual(
             self.get_constraints_for_column(BookWithoutAuthor, 'title'),
-            ['schema_book_d5d3db17', 'schema_book_title_2dfb2dff_like']
+            ['schema_book_title_2dfb2dff', 'schema_book_title_2dfb2dff_like']
         )
 
     @unittest.skipUnless(connection.vendor == 'postgresql', "PostgreSQL specific")
@@ -2032,7 +2032,7 @@ class SchemaTests(TransactionTestCase):
             editor.create_model(BookWithoutAuthor)
         self.assertEqual(
             self.get_constraints_for_column(BookWithoutAuthor, 'title'),
-            ['schema_book_d5d3db17', 'schema_book_title_2dfb2dff_like']
+            ['schema_book_title_2dfb2dff', 'schema_book_title_2dfb2dff_like']
         )
         # Alter to add unique=True (should replace the index)
         old_field = BookWithoutAuthor._meta.get_field('title')
@@ -2058,7 +2058,7 @@ class SchemaTests(TransactionTestCase):
             editor.create_model(BookWithoutAuthor)
         self.assertEqual(
             self.get_constraints_for_column(BookWithoutAuthor, 'title'),
-            ['schema_book_d5d3db17', 'schema_book_title_2dfb2dff_like']
+            ['schema_book_title_2dfb2dff', 'schema_book_title_2dfb2dff_like']
         )
         # Alter to set unique=True and remove db_index=True (should replace the index)
         old_field = BookWithoutAuthor._meta.get_field('title')
@@ -2077,7 +2077,7 @@ class SchemaTests(TransactionTestCase):
             editor.alter_field(BookWithoutAuthor, new_field, new_field2, strict=True)
         self.assertEqual(
             self.get_constraints_for_column(BookWithoutAuthor, 'title'),
-            ['schema_book_d5d3db17', 'schema_book_title_2dfb2dff_like']
+            ['schema_book_title_2dfb2dff', 'schema_book_title_2dfb2dff_like']
         )
 
     @unittest.skipUnless(connection.vendor == 'postgresql', "PostgreSQL specific")
@@ -2122,7 +2122,7 @@ class SchemaTests(TransactionTestCase):
         with connection.schema_editor() as editor:
             editor.alter_field(Author, old_field, new_field, strict=True)
 
-        expected = 'schema_author_7edabf99'
+        expected = 'schema_author_weight_587740f9'
         if connection.features.uppercases_column_names:
             expected = expected.upper()
         self.assertEqual(self.get_constraints_for_column(Author, 'weight'), [expected])