From 3a65e5783b5d88c2f5b78fe09de240c3dd3f9f90 Mon Sep 17 00:00:00 2001 From: Tom Sparrow <793763+sparrowt@users.noreply.github.com> Date: Tue, 25 Feb 2020 16:31:27 +0000 Subject: [PATCH 1/2] Fix DROP index vs constraint issue (#38) `_constraint_names` return value includes unique indexes even if these are not actual constraints. Trying to drop these using `sql_delete_unique` (ALTER TABLE ... DROP CONSTRAINT ...) will fail because it is an index, not a constraint. Instead explicitly deal with unique indexes separately. --- sql_server/pyodbc/schema.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sql_server/pyodbc/schema.py b/sql_server/pyodbc/schema.py index 06ae7caa..2bd2dcc5 100644 --- a/sql_server/pyodbc/schema.py +++ b/sql_server/pyodbc/schema.py @@ -580,15 +580,21 @@ def _delete_unique_constraints(self, model, old_field, new_field, strict=False): unique_columns.append([old_field.column]) if unique_columns: for columns in unique_columns: - constraint_names = self._constraint_names(model, columns, unique=True) + constraint_names_normal = self._constraint_names(model, columns, unique=True, index=False) + constraint_names_index = self._constraint_names(model, columns, unique=True, index=True) + constraint_names = constraint_names_normal + constraint_names_index if strict and len(constraint_names) != 1: raise ValueError("Found wrong number (%s) of unique constraints for %s.%s" % ( len(constraint_names), model._meta.db_table, old_field.column, )) - for constraint_name in constraint_names: + for constraint_name in constraint_names_normal: self.execute(self._delete_constraint_sql(self.sql_delete_unique, model, constraint_name)) + # Unique indexes which are not table constraints must be deleted using the appropriate SQL. + # These may exist for example to enforce ANSI-compliant unique constraints on nullable columns. + for index_name in constraint_names_index: + self.execute(self._delete_constraint_sql(self.sql_delete_index, model, index_name)) def _rename_field_sql(self, table, old_field, new_field, new_type): new_type = self._set_field_new_type_null_status(old_field, new_type) From 66b4e08488ec251fca7b32c4f347c5228ddd79cc Mon Sep 17 00:00:00 2001 From: Tom Sparrow <793763+sparrowt@users.noreply.github.com> Date: Mon, 9 Mar 2020 10:37:26 +0000 Subject: [PATCH 2/2] Add testapp migrations as regression test for issue (#38) --- .../0002_test_unique_nullable_part1.py | 19 +++++++++++++++++++ .../0003_test_unique_nullable_part2.py | 18 ++++++++++++++++++ testapp/models.py | 7 +++++++ 3 files changed, 44 insertions(+) create mode 100644 testapp/migrations/0002_test_unique_nullable_part1.py create mode 100644 testapp/migrations/0003_test_unique_nullable_part2.py diff --git a/testapp/migrations/0002_test_unique_nullable_part1.py b/testapp/migrations/0002_test_unique_nullable_part1.py new file mode 100644 index 00000000..1c0e48d2 --- /dev/null +++ b/testapp/migrations/0002_test_unique_nullable_part1.py @@ -0,0 +1,19 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('testapp', '0001_initial'), + ] + + operations = [ + # Create with a field that is unique *and* nullable so it is implemented with a filtered unique index. + migrations.CreateModel( + name='TestUniqueNullableModel', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('test_field', models.CharField(max_length=100, null=True, unique=True)), + ], + ), + ] diff --git a/testapp/migrations/0003_test_unique_nullable_part2.py b/testapp/migrations/0003_test_unique_nullable_part2.py new file mode 100644 index 00000000..d6fc61e0 --- /dev/null +++ b/testapp/migrations/0003_test_unique_nullable_part2.py @@ -0,0 +1,18 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('testapp', '0002_test_unique_nullable_part1'), + ] + + operations = [ + # Now remove the null=True to check this transition is correctly handled. + migrations.AlterField( + model_name='testuniquenullablemodel', + name='test_field', + field=models.CharField(default='', max_length=100, unique=True), + preserve_default=False, + ), + ] diff --git a/testapp/models.py b/testapp/models.py index d177d706..503d81ce 100644 --- a/testapp/models.py +++ b/testapp/models.py @@ -41,3 +41,10 @@ class UUIDModel(models.Model): def __str__(self): return self.pk + + +class TestUniqueNullableModel(models.Model): + # This field started off as unique=True *and* null=True so it is implemented with a filtered unique index + # Then it is made non-nullable by a subsequent migration, to check this is correctly handled (the index + # should be dropped, then a normal unique constraint should be added, now that the column is not nullable) + test_field = models.CharField(max_length=100, unique=True)