From 831358f23d545b8dba017c6b26bd295ba9f6c17d Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Thu, 3 Aug 2017 23:33:06 +0200 Subject: [PATCH] Fixed #28465 -- Unified index SQL creation in DatabaseSchemaEditor Thanks Tim Graham for the review. --- .../contrib/gis/db/backends/postgis/schema.py | 4 +-- django/contrib/postgres/indexes.py | 22 ++++++++-------- django/db/backends/base/schema.py | 14 +++++++---- django/db/backends/ddl_references.py | 11 ++++++-- django/db/models/indexes.py | 25 +++++-------------- tests/model_indexes/tests.py | 8 +++--- 6 files changed, 40 insertions(+), 44 deletions(-) diff --git a/django/contrib/gis/db/backends/postgis/schema.py b/django/contrib/gis/db/backends/postgis/schema.py index 7a7f88f02d..e64a108681 100644 --- a/django/contrib/gis/db/backends/postgis/schema.py +++ b/django/contrib/gis/db/backends/postgis/schema.py @@ -17,9 +17,9 @@ class PostGISSchemaEditor(DatabaseSchemaEditor): return True return super()._field_should_be_indexed(model, field) - def _create_index_sql(self, model, fields, suffix="", sql=None): + def _create_index_sql(self, model, fields, **kwargs): if len(fields) != 1 or not hasattr(fields[0], 'geodetic'): - return super()._create_index_sql(model, fields, suffix=suffix, sql=sql) + return super()._create_index_sql(model, fields, **kwargs) field = fields[0] field_column = self.quote_name(field.column) diff --git a/django/contrib/postgres/indexes.py b/django/contrib/postgres/indexes.py index b7ca1d3dc4..5ece631829 100644 --- a/django/contrib/postgres/indexes.py +++ b/django/contrib/postgres/indexes.py @@ -22,12 +22,13 @@ class BrinIndex(Index): kwargs['pages_per_range'] = self.pages_per_range return path, args, kwargs - def get_sql_create_template_values(self, model, schema_editor, using): - parameters = super().get_sql_create_template_values(model, schema_editor, using=' USING brin') + def create_sql(self, model, schema_editor, using=''): + statement = super().create_sql(model, schema_editor, using=' USING brin') if self.pages_per_range is not None: - parameters['extra'] = ' WITH (pages_per_range={})'.format( - schema_editor.quote_value(self.pages_per_range)) + parameters['extra'] - return parameters + statement.parts['extra'] = ' WITH (pages_per_range={})'.format( + schema_editor.quote_value(self.pages_per_range) + ) + statement.parts['extra'] + return statement class GinIndex(Index): @@ -44,16 +45,13 @@ class GinIndex(Index): kwargs['gin_pending_list_limit'] = self.gin_pending_list_limit return path, args, kwargs - def get_sql_create_template_values(self, model, schema_editor, using): - parameters = super().get_sql_create_template_values(model, schema_editor, using=' USING gin') + def create_sql(self, model, schema_editor, using=''): + statement = super().create_sql(model, schema_editor, using=' USING gin') with_params = [] if self.gin_pending_list_limit is not None: with_params.append('gin_pending_list_limit = %d' % self.gin_pending_list_limit) if self.fastupdate is not None: with_params.append('fastupdate = {}'.format('on' if self.fastupdate else 'off')) if with_params: - parameters['extra'] = 'WITH ({}) {}'.format(', '.join(with_params), parameters['extra']) - return parameters - - def create_sql(self, model, schema_editor): - return super().create_sql(model, schema_editor, using=' USING gin') + statement.parts['extra'] = 'WITH ({}) {}'.format(', '.join(with_params), statement.parts['extra']) + return statement diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 2d468a8c66..479d52ccaa 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -889,26 +889,30 @@ class BaseDatabaseSchemaEditor: return ' ' + self.connection.ops.tablespace_sql(db_tablespace) return '' - def _create_index_sql(self, model, fields, suffix="", sql=None): + def _create_index_sql(self, model, fields, *, name=None, suffix='', using='', + db_tablespace=None, col_suffixes=(), sql=None): """ Return the SQL statement to create the index for one or several fields. `sql` can be specified if the syntax differs from the standard (GIS indexes, ...). """ - tablespace_sql = self._get_index_tablespace_sql(model, fields) + tablespace_sql = self._get_index_tablespace_sql(model, fields, db_tablespace=db_tablespace) columns = [field.column for field in fields] sql_create_index = sql or self.sql_create_index table = model._meta.db_table def create_index_name(*args, **kwargs): - return self.quote_name(self._create_index_name(*args, **kwargs)) + nonlocal name + if name is None: + name = self._create_index_name(*args, **kwargs) + return self.quote_name(name) return Statement( sql_create_index, table=Table(table, self.quote_name), name=IndexName(table, columns, suffix, create_index_name), - using='', - columns=Columns(table, columns, self.quote_name), + using=using, + columns=Columns(table, columns, self.quote_name, col_suffixes=col_suffixes), extra=tablespace_sql, ) diff --git a/django/db/backends/ddl_references.py b/django/db/backends/ddl_references.py index 61b7b9eaf8..b894d58793 100644 --- a/django/db/backends/ddl_references.py +++ b/django/db/backends/ddl_references.py @@ -76,12 +76,19 @@ class TableColumns(Table): class Columns(TableColumns): """Hold a reference to one or many columns.""" - def __init__(self, table, columns, quote_name): + def __init__(self, table, columns, quote_name, col_suffixes=()): self.quote_name = quote_name + self.col_suffixes = col_suffixes super().__init__(table, columns) def __str__(self): - return ', '.join(self.quote_name(column) for column in self.columns) + def col_str(column, idx): + try: + return self.quote_name(column) + self.col_suffixes[idx] + except IndexError: + return self.quote_name(column) + + return ', '.join(col_str(column, idx) for idx, column in enumerate(self.columns)) class IndexName(TableColumns): diff --git a/django/db/models/indexes.py b/django/db/models/indexes.py index b4fc36a265..73c516532e 100644 --- a/django/db/models/indexes.py +++ b/django/db/models/indexes.py @@ -43,26 +43,13 @@ class Index: self.name = 'D%s' % self.name[1:] return errors - def get_sql_create_template_values(self, model, schema_editor, using): - fields = [model._meta.get_field(field_name) for field_name, order in self.fields_orders] - tablespace_sql = schema_editor._get_index_tablespace_sql(model, fields, self.db_tablespace) - quote_name = schema_editor.quote_name - columns = [ - ('%s %s' % (quote_name(field.column), order)).strip() - for field, (field_name, order) in zip(fields, self.fields_orders) - ] - return { - 'table': quote_name(model._meta.db_table), - 'name': quote_name(self.name), - 'columns': ', '.join(columns), - 'using': using, - 'extra': tablespace_sql, - } - def create_sql(self, model, schema_editor, using=''): - sql_create_index = schema_editor.sql_create_index - sql_parameters = self.get_sql_create_template_values(model, schema_editor, using) - return sql_create_index % sql_parameters + fields = [model._meta.get_field(field_name) for field_name, _ in self.fields_orders] + col_suffixes = [order[1] for order in self.fields_orders] + return schema_editor._create_index_sql( + model, fields, name=self.name, using=using, db_tablespace=self.db_tablespace, + col_suffixes=col_suffixes, + ) def remove_sql(self, model, schema_editor): quote_name = schema_editor.quote_name diff --git a/tests/model_indexes/tests.py b/tests/model_indexes/tests.py index 555b0bb0aa..381f4fdcf8 100644 --- a/tests/model_indexes/tests.py +++ b/tests/model_indexes/tests.py @@ -113,7 +113,7 @@ class IndexesTests(SimpleTestCase): ]: with self.subTest(fields=fields): index = models.Index(fields=fields, db_tablespace='idx_tbls2') - self.assertIn('"idx_tbls2"', index.create_sql(Book, editor).lower()) + self.assertIn('"idx_tbls2"', str(index.create_sql(Book, editor)).lower()) # Indexes without db_tablespace attribute. for fields in [['author'], ['shortcut', 'isbn'], ['title', 'author']]: with self.subTest(fields=fields): @@ -124,11 +124,11 @@ class IndexesTests(SimpleTestCase): if settings.DEFAULT_INDEX_TABLESPACE: self.assertIn( '"%s"' % settings.DEFAULT_INDEX_TABLESPACE, - index.create_sql(Book, editor).lower() + str(index.create_sql(Book, editor)).lower() ) else: - self.assertNotIn('TABLESPACE', index.create_sql(Book, editor)) + self.assertNotIn('TABLESPACE', str(index.create_sql(Book, editor))) # Field with db_tablespace specified on the model and an index # without db_tablespace. index = models.Index(fields=['shortcut']) - self.assertIn('"idx_tbls"', index.create_sql(Book, editor).lower()) + self.assertIn('"idx_tbls"', str(index.create_sql(Book, editor)).lower())