From ddb5674945725ec8f6e6336f73d3d56e331f34ae Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Wed, 16 Jul 2014 11:06:33 +0200 Subject: [PATCH] [1.7.x] Fixed #23030 -- Properly handled geometry columns metadata during migrations Thanks kunitoki for the report and initial patches. Backport of 8c30df15f1 from master. --- .../contrib/gis/db/backends/postgis/models.py | 3 ++- .../gis/db/backends/spatialite/schema.py | 17 +++++++------- .../gis_migrations/migrations/0001_initial.py | 23 ++++++++++++++++--- .../gis/tests/gis_migrations/test_commands.py | 20 ++++++++++++++++ django/db/backends/sqlite3/schema.py | 18 ++++++++++----- 5 files changed, 63 insertions(+), 18 deletions(-) diff --git a/django/contrib/gis/db/backends/postgis/models.py b/django/contrib/gis/db/backends/postgis/models.py index 2dae1529ec..3916ec4f7f 100644 --- a/django/contrib/gis/db/backends/postgis/models.py +++ b/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) diff --git a/django/contrib/gis/db/backends/spatialite/schema.py b/django/contrib/gis/db/backends/spatialite/schema.py index b1283c39af..673a08900c 100644 --- a/django/contrib/gis/db/backends/spatialite/schema.py +++ b/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)), + }) diff --git a/django/contrib/gis/tests/gis_migrations/migrations/0001_initial.py b/django/contrib/gis/tests/gis_migrations/migrations/0001_initial.py index 003a4d6178..a808e49444 100644 --- a/django/contrib/gis/tests/gis_migrations/migrations/0001_initial.py +++ b/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, + ), ] diff --git a/django/contrib/gis/tests/gis_migrations/test_commands.py b/django/contrib/gis/tests/gis_migrations/test_commands.py index f927613511..e2b4367a6b 100644 --- a/django/contrib/gis/tests/gis_migrations/test_commands.py +++ b/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) diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index 3e122d707d..85aaaa1af4 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/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.