From 4bb859e24694f6cb8974ed9d2225f18214338ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pave=C5=82=20Ty=C5=9Blacki?= Date: Mon, 11 Feb 2019 17:17:06 +0300 Subject: [PATCH] Refs #30172 -- Prevented removing a field's check or unique constraint from removing Meta constraints. --- django/db/backends/base/features.py | 4 + django/db/backends/base/schema.py | 17 ++++- django/db/backends/oracle/features.py | 1 + tests/schema/models.py | 7 ++ tests/schema/tests.py | 104 +++++++++++++++++++++++++- 5 files changed, 125 insertions(+), 8 deletions(-) diff --git a/django/db/backends/base/features.py b/django/db/backends/base/features.py index 31e969a95c3..3417fdf1cde 100644 --- a/django/db/backends/base/features.py +++ b/django/db/backends/base/features.py @@ -281,6 +281,10 @@ class BaseDatabaseFeatures: supports_partial_indexes = True supports_functions_in_partial_indexes = True + # Does the database allow more than one constraint or index on the same + # field(s)? + allows_multiple_constraints_on_same_fields = True + def __init__(self, connection): self.connection = connection diff --git a/django/db/backends/base/schema.py b/django/db/backends/base/schema.py index 0522abcaadc..35ab9bf2190 100644 --- a/django/db/backends/base/schema.py +++ b/django/db/backends/base/schema.py @@ -562,7 +562,11 @@ class BaseDatabaseSchemaEditor: # Has unique been removed? if old_field.unique and (not new_field.unique or self._field_became_primary_key(old_field, new_field)): # Find the unique constraint for this field - constraint_names = self._constraint_names(model, [old_field.column], unique=True, primary_key=False) + meta_constraint_names = {constraint.name for constraint in model._meta.constraints} + constraint_names = self._constraint_names( + model, [old_field.column], unique=True, primary_key=False, + exclude=meta_constraint_names, + ) if strict and len(constraint_names) != 1: raise ValueError("Found wrong number (%s) of unique constraints for %s.%s" % ( len(constraint_names), @@ -612,7 +616,11 @@ class BaseDatabaseSchemaEditor: self.execute(self._delete_index_sql(model, index_name)) # Change check constraints? if old_db_params['check'] != new_db_params['check'] and old_db_params['check']: - constraint_names = self._constraint_names(model, [old_field.column], check=True) + meta_constraint_names = {constraint.name for constraint in model._meta.constraints} + constraint_names = self._constraint_names( + model, [old_field.column], check=True, + exclude=meta_constraint_names, + ) if strict and len(constraint_names) != 1: raise ValueError("Found wrong number (%s) of check constraints for %s.%s" % ( len(constraint_names), @@ -1106,7 +1114,7 @@ class BaseDatabaseSchemaEditor: def _constraint_names(self, model, column_names=None, unique=None, primary_key=None, index=None, foreign_key=None, - check=None, type_=None): + check=None, type_=None, exclude=None): """Return all constraint names matching the columns and conditions.""" if column_names is not None: column_names = [ @@ -1130,7 +1138,8 @@ class BaseDatabaseSchemaEditor: continue if type_ is not None and infodict['type'] != type_: continue - result.append(name) + if not exclude or name not in exclude: + result.append(name) return result def _delete_primary_key(self, model, strict=False): diff --git a/django/db/backends/oracle/features.py b/django/db/backends/oracle/features.py index 6c0fa6266d2..2da11355ed4 100644 --- a/django/db/backends/oracle/features.py +++ b/django/db/backends/oracle/features.py @@ -57,3 +57,4 @@ class DatabaseFeatures(BaseDatabaseFeatures): max_query_params = 2**16 - 1 supports_partial_indexes = False supports_slicing_ordering_in_compound = True + allows_multiple_constraints_on_same_fields = False diff --git a/tests/schema/models.py b/tests/schema/models.py index f6bdbd88157..2c277c681bc 100644 --- a/tests/schema/models.py +++ b/tests/schema/models.py @@ -55,6 +55,13 @@ class AuthorWithIndexedName(models.Model): apps = new_apps +class AuthorWithUniqueName(models.Model): + name = models.CharField(max_length=255, unique=True) + + class Meta: + apps = new_apps + + class Book(models.Model): author = models.ForeignKey(Author, models.CASCADE) title = models.CharField(max_length=100, db_index=True) diff --git a/tests/schema/tests.py b/tests/schema/tests.py index 00ce2e494e2..050e4c9385d 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -7,7 +7,8 @@ from unittest import mock from django.db import ( DatabaseError, IntegrityError, OperationalError, connection, ) -from django.db.models import Model +from django.db.models import Model, Q +from django.db.models.constraints import CheckConstraint, UniqueConstraint from django.db.models.deletion import CASCADE, PROTECT from django.db.models.fields import ( AutoField, BigAutoField, BigIntegerField, BinaryField, BooleanField, @@ -31,9 +32,10 @@ from .fields import ( from .models import ( Author, AuthorCharFieldWithIndex, AuthorTextFieldWithIndex, AuthorWithDefaultHeight, AuthorWithEvenLongerName, AuthorWithIndexedName, - Book, BookForeignObj, BookWeak, BookWithLongName, BookWithO2O, - BookWithoutAuthor, BookWithSlug, IntegerPK, Node, Note, NoteRename, Tag, - TagIndexed, TagM2MTest, TagUniqueRename, Thing, UniqueTest, new_apps, + AuthorWithUniqueName, Book, BookForeignObj, BookWeak, BookWithLongName, + BookWithO2O, BookWithoutAuthor, BookWithSlug, IntegerPK, Node, Note, + NoteRename, Tag, TagIndexed, TagM2MTest, TagUniqueRename, Thing, + UniqueTest, new_apps, ) @@ -1545,6 +1547,53 @@ class SchemaTests(TransactionTestCase): if not any(details['columns'] == ['height'] and details['check'] for details in constraints.values()): self.fail("No check constraint for height found") + @skipUnlessDBFeature('supports_column_check_constraints') + def test_remove_field_check_does_not_remove_meta_constraints(self): + with connection.schema_editor() as editor: + editor.create_model(Author) + # Add the custom check constraint + constraint = CheckConstraint(check=Q(height__gte=0), name='author_height_gte_0_check') + custom_constraint_name = constraint.name + Author._meta.constraints = [constraint] + with connection.schema_editor() as editor: + editor.add_constraint(Author, constraint) + # Ensure the constraints exist + constraints = self.get_constraints(Author._meta.db_table) + self.assertIn(custom_constraint_name, constraints) + other_constraints = [ + name for name, details in constraints.items() + if details['columns'] == ['height'] and details['check'] and name != custom_constraint_name + ] + self.assertEqual(len(other_constraints), 1) + # Alter the column to remove field check + old_field = Author._meta.get_field('height') + new_field = IntegerField(null=True, blank=True) + new_field.set_attributes_from_name('height') + with connection.schema_editor() as editor: + editor.alter_field(Author, old_field, new_field, strict=True) + constraints = self.get_constraints(Author._meta.db_table) + self.assertIn(custom_constraint_name, constraints) + other_constraints = [ + name for name, details in constraints.items() + if details['columns'] == ['height'] and details['check'] and name != custom_constraint_name + ] + self.assertEqual(len(other_constraints), 0) + # Alter the column to re-add field check + new_field2 = Author._meta.get_field('height') + with connection.schema_editor() as editor: + editor.alter_field(Author, new_field, new_field2, strict=True) + constraints = self.get_constraints(Author._meta.db_table) + self.assertIn(custom_constraint_name, constraints) + other_constraints = [ + name for name, details in constraints.items() + if details['columns'] == ['height'] and details['check'] and name != custom_constraint_name + ] + self.assertEqual(len(other_constraints), 1) + # Drop the check constraint + with connection.schema_editor() as editor: + Author._meta.constraints = [] + editor.remove_constraint(Author, constraint) + def test_unique(self): """ Tests removing and adding unique constraints to a single column. @@ -1671,6 +1720,53 @@ class SchemaTests(TransactionTestCase): with self.assertRaises(IntegrityError): Tag.objects.create(title='bar', slug='foo') + @skipUnlessDBFeature('allows_multiple_constraints_on_same_fields') + def test_remove_field_unique_does_not_remove_meta_constraints(self): + with connection.schema_editor() as editor: + editor.create_model(AuthorWithUniqueName) + # Add the custom unique constraint + constraint = UniqueConstraint(fields=['name'], name='author_name_uniq') + custom_constraint_name = constraint.name + AuthorWithUniqueName._meta.constraints = [constraint] + with connection.schema_editor() as editor: + editor.add_constraint(AuthorWithUniqueName, constraint) + # Ensure the constraints exist + constraints = self.get_constraints(AuthorWithUniqueName._meta.db_table) + self.assertIn(custom_constraint_name, constraints) + other_constraints = [ + name for name, details in constraints.items() + if details['columns'] == ['name'] and details['unique'] and name != custom_constraint_name + ] + self.assertEqual(len(other_constraints), 1) + # Alter the column to remove field uniqueness + old_field = AuthorWithUniqueName._meta.get_field('name') + new_field = CharField(max_length=255) + new_field.set_attributes_from_name('name') + with connection.schema_editor() as editor: + editor.alter_field(AuthorWithUniqueName, old_field, new_field, strict=True) + constraints = self.get_constraints(AuthorWithUniqueName._meta.db_table) + self.assertIn(custom_constraint_name, constraints) + other_constraints = [ + name for name, details in constraints.items() + if details['columns'] == ['name'] and details['unique'] and name != custom_constraint_name + ] + self.assertEqual(len(other_constraints), 0) + # Alter the column to re-add field uniqueness + new_field2 = AuthorWithUniqueName._meta.get_field('name') + with connection.schema_editor() as editor: + editor.alter_field(AuthorWithUniqueName, new_field, new_field2, strict=True) + constraints = self.get_constraints(AuthorWithUniqueName._meta.db_table) + self.assertIn(custom_constraint_name, constraints) + other_constraints = [ + name for name, details in constraints.items() + if details['columns'] == ['name'] and details['unique'] and name != custom_constraint_name + ] + self.assertEqual(len(other_constraints), 1) + # Drop the unique constraint + with connection.schema_editor() as editor: + AuthorWithUniqueName._meta.constraints = [] + editor.remove_constraint(AuthorWithUniqueName, constraint) + def test_unique_together(self): """ Tests removing and adding unique_together constraints on a model.