Fixed #23030 -- Properly handled geometry columns metadata during migrations

Thanks kunitoki for the report and initial patches.

django/contrib/gis/db/backends/postgis/models.py

@@ -10,7 +10,8 @@ from django.utils.encoding import python_2_unicode_compatible
 class PostGISGeometryColumns(models.Model):
     """
     The 'geometry_columns' table from the PostGIS. See the PostGIS
-    documentation at Ch. 4.2.2.
+    documentation at Ch. 4.3.2.
+    On PostGIS 2, this is a view.
     """
     f_table_catalog = models.CharField(max_length=256)
     f_table_schema = models.CharField(max_length=256)

django/contrib/gis/db/backends/spatialite/schema.py

@@ -62,13 +62,13 @@ class SpatialiteSchemaEditor(DatabaseSchemaEditor):
             self.execute(sql)
         self.geometry_sql = []
 
-    def delete_model(self, model):
+    def delete_model(self, model, **kwargs):
         from django.contrib.gis.db.models.fields import GeometryField
         # Drop spatial metadata (dropping the table does not automatically remove them)
         for field in model._meta.local_fields:
             if isinstance(field, GeometryField):
                 self.remove_geometry_metadata(model, field)
-        super(SpatialiteSchemaEditor, self).delete_model(model)
+        super(SpatialiteSchemaEditor, self).delete_model(model, **kwargs)
 
     def add_field(self, model, field):
         from django.contrib.gis.db.models.fields import GeometryField
@@ -81,12 +81,6 @@ class SpatialiteSchemaEditor(DatabaseSchemaEditor):
         else:
             super(SpatialiteSchemaEditor, self).add_field(model, field)
 
-    def remove_field(self, model, field):
-        from django.contrib.gis.db.models.fields import GeometryField
-        if isinstance(field, GeometryField):
-            self.remove_geometry_metadata(model, field)
-        super(SpatialiteSchemaEditor, self).remove_field(model, field)
-
     def alter_db_table(self, model, old_db_table, new_db_table):
         super(SpatialiteSchemaEditor, self).alter_db_table(model, old_db_table, new_db_table)
         self.execute(
@@ -95,3 +89,10 @@ class SpatialiteSchemaEditor(DatabaseSchemaEditor):
                 "new_table": self.quote_name(new_db_table),
             }
         )
+        # Also rename spatial index tables
+        for field in model._meta.local_fields:
+            if getattr(field, 'spatial_index', False):
+                self.execute(self.sql_rename_table % {
+                    "old_table": self.quote_name("idx_%s_%s" % (old_db_table, field.column)),
+                    "new_table": self.quote_name("idx_%s_%s" % (new_db_table, field.column)),
+                })

django/contrib/gis/tests/gis_migrations/migrations/0001_initial.py

@@ -2,9 +2,10 @@ from django.db import models, migrations
 import django.contrib.gis.db.models.fields
 
 
-# Used for regression test of ticket #22001: https://code.djangoproject.com/ticket/22001
 class Migration(migrations.Migration):
-
+    """
+    Used for gis.specific migration tests.
+    """
     operations = [
         migrations.CreateModel(
             name='Neighborhood',
@@ -29,5 +30,21 @@ class Migration(migrations.Migration):
             options={
             },
             bases=(models.Model,),
-        )
+        ),
+        migrations.CreateModel(
+            name='Family',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('name', models.CharField(max_length=100, unique=True)),
+            ],
+            options={
+            },
+            bases=(models.Model,),
+        ),
+        migrations.AddField(
+            model_name='household',
+            name='family',
+            field=models.ForeignKey(blank=True, to='gis.Family', null=True),
+            preserve_default=True,
+        ),
     ]

django/contrib/gis/tests/gis_migrations/test_commands.py

@@ -34,17 +34,37 @@ class MigrateTests(TransactionTestCase):
         Tests basic usage of the migrate command when a model uses Geodjango
         fields. Regression test for ticket #22001:
         https://code.djangoproject.com/ticket/22001
+
+        It's also used to showcase an error in migrations where spatialite is
+        enabled and geo tables are renamed resulting in unique constraint
+        failure on geometry_columns. Regression for ticket #23030:
+        https://code.djangoproject.com/ticket/23030
         """
         # Make sure no tables are created
         self.assertTableNotExists("migrations_neighborhood")
         self.assertTableNotExists("migrations_household")
+        self.assertTableNotExists("migrations_family")
         # Run the migrations to 0001 only
         call_command("migrate", "gis", "0001", verbosity=0)
         # Make sure the right tables exist
         self.assertTableExists("gis_neighborhood")
         self.assertTableExists("gis_household")
+        self.assertTableExists("gis_family")
         # Unmigrate everything
         call_command("migrate", "gis", "zero", verbosity=0)
         # Make sure it's all gone
         self.assertTableNotExists("gis_neighborhood")
         self.assertTableNotExists("gis_household")
+        self.assertTableNotExists("gis_family")
+        # Even geometry columns metadata
+        try:
+            GeoColumn = connection.ops.geometry_columns()
+        except NotImplementedError:
+            # Not all GIS backends have geometry columns model
+            pass
+        else:
+            self.assertEqual(
+                GeoColumn.objects.filter(
+                    **{'%s__in' % GeoColumn.table_name_col(): ["gis_neighborhood", "gis_household"]}
+                    ).count(),
+                0)

django/db/backends/sqlite3/schema.py

@@ -127,13 +127,10 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
             ', '.join(y for x, y in field_maps),
             self.quote_name(model._meta.db_table),
         ))
-        # Delete the old table (not using self.delete_model to avoid deleting
-        # all implicit M2M tables)
-        self.execute(self.sql_delete_table % {
-            "table": self.quote_name(model._meta.db_table),
-        })
+        # Delete the old table
+        self.delete_model(model, handle_autom2m=False)
         # Rename the new to the old
-        self.alter_db_table(model, temp_model._meta.db_table, model._meta.db_table)
+        self.alter_db_table(temp_model, temp_model._meta.db_table, model._meta.db_table)
         # Run deferred SQL on correct table
         for sql in self.deferred_sql:
             self.execute(sql.replace(temp_model._meta.db_table, model._meta.db_table))
@@ -142,6 +139,15 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
         if restore_pk_field:
             restore_pk_field.primary_key = True
 
+    def delete_model(self, model, handle_autom2m=True):
+        if handle_autom2m:
+            super(DatabaseSchemaEditor, self).delete_model(model)
+        else:
+            # Delete the table (and only that)
+            self.execute(self.sql_delete_table % {
+                "table": self.quote_name(model._meta.db_table),
+            })
+
     def add_field(self, model, field):
         """
         Creates a field on a model.