Fixed #28465 -- Unified index SQL creation in DatabaseSchemaEditor

Thanks Tim Graham for the review.

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)

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):
+    def create_sql(self, model, schema_editor, using=''):
-        parameters = super().get_sql_create_template_values(model, schema_editor, using=' USING brin')
+        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(
+            statement.parts['extra'] = ' WITH (pages_per_range={})'.format(
-                schema_editor.quote_value(self.pages_per_range)) + parameters['extra']
+                schema_editor.quote_value(self.pages_per_range)
-        return parameters
+            ) + 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):
+    def create_sql(self, model, schema_editor, using=''):
-        parameters = super().get_sql_create_template_values(model, schema_editor, using=' USING gin')
+        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'])
+            statement.parts['extra'] = 'WITH ({}) {}'.format(', '.join(with_params), statement.parts['extra'])
-        return parameters
+        return statement
-
-    def create_sql(self, model, schema_editor):
-        return super().create_sql(model, schema_editor, using=' USING gin')

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='',
+            using=using,
-            columns=Columns(table, columns, self.quote_name),
+            columns=Columns(table, columns, self.quote_name, col_suffixes=col_suffixes),
             extra=tablespace_sql,
         )
 

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):

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
+        fields = [model._meta.get_field(field_name) for field_name, _ in self.fields_orders]
-        sql_parameters = self.get_sql_create_template_values(model, schema_editor, using)
+        col_suffixes = [order[1] for order in self.fields_orders]
-        return sql_create_index % sql_parameters
+        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

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())