From 324c1b432a2fa07c8f911f8b923ed04be32c52c7 Mon Sep 17 00:00:00 2001 From: Akshesh Date: Fri, 8 Jul 2016 19:41:19 +0530 Subject: [PATCH] Fixed #24442 -- Improved SchemaEditor's index name truncation. --- django/db/backends/base/schema.py | 47 ++++++++++++++---------------- tests/indexes/tests.py | 30 +++++++++++++++++-- tests/postgres_tests/test_array.py | 10 +++---- tests/schema/tests.py | 18 ++++++------ 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])