Fixed #30172 -- Prevented Meta constraints from being removed in other migration operations. #10978

tbicr · 2019-02-11T14:27:23Z

https://code.djangoproject.com/ticket/30172

tbicr · 2019-02-12T22:19:33Z

issue still actual for unique constraints for sqlite, that related to sqlite introspection bug: https://code.djangoproject.com/ticket/30183

auvipy

fix the flake8 error plz

tbicr · 2019-03-11T08:55:19Z

fix the flake8 error plz

fixed

timgraham · 2019-03-13T14:40:08Z

Is #11005 no longer a dependency of this?

timgraham · 2019-03-13T17:24:31Z

Oracle errors:

======================================================================
ERROR: test_remove_field_uniqueness_does_not_remove_custom_constraints (schema.tests.SchemaTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 510, in execute
    return self.cursor.execute(query, self._param_generator(params))
cx_Oracle.DatabaseError: ORA-02261: such unique or primary key already exists in the table

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tim/code/django/tests/schema/tests.py", line 1739, in test_remove_field_uniqueness_does_not_remove_custom_constraints
    editor.add_constraint(AuthorWithUniqueName, constraint)
  File "/home/tim/code/django/django/db/backends/base/schema.py", line 346, in add_constraint
    self.execute(sql)
  File "/home/tim/code/django/django/db/backends/base/schema.py", line 138, in execute
    cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/home/tim/code/django/django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 510, in execute
    return self.cursor.execute(query, self._param_generator(params))
django.db.utils.DatabaseError: ORA-02261: such unique or primary key already exists in the table

======================================================================
ERROR: test_remove_index_together_does_not_remove_custom_indexes (schema.tests.SchemaTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/django/db/backends/utils.py", line 82, in _execute
    return self.cursor.execute(sql)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 510, in execute
    return self.cursor.execute(query, self._param_generator(params))
cx_Oracle.DatabaseError: ORA-01408: such column list already indexed

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tim/code/django/tests/schema/tests.py", line 1998, in test_remove_index_together_does_not_remove_custom_indexes
    editor.add_index(AuthorWithIndexedNameAndBirthday, index)
  File "/home/tim/code/django/django/db/backends/base/schema.py", line 336, in add_index
    self.execute(index.create_sql(model, self), params=None)
  File "/home/tim/code/django/django/db/backends/base/schema.py", line 138, in execute
    cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/home/tim/code/django/django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/tim/code/django/django/db/backends/utils.py", line 82, in _execute
    return self.cursor.execute(sql)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 510, in execute
    return self.cursor.execute(query, self._param_generator(params))
django.db.utils.DatabaseError: ORA-01408: such column list already indexed

======================================================================
ERROR: test_remove_unique_together_does_not_remove_custom_constraints (schema.tests.SchemaTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 510, in execute
    return self.cursor.execute(query, self._param_generator(params))
cx_Oracle.DatabaseError: ORA-02261: such unique or primary key already exists in the table

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tim/code/django/tests/schema/tests.py", line 1866, in test_remove_unique_together_does_not_remove_custom_constraints
    editor.add_constraint(AuthorWithUniqueNameAndBirthday, constraint)
  File "/home/tim/code/django/django/db/backends/base/schema.py", line 346, in add_constraint
    self.execute(sql)
  File "/home/tim/code/django/django/db/backends/base/schema.py", line 138, in execute
    cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/home/tim/code/django/django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/tim/code/django/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/tim/code/django/django/db/backends/oracle/base.py", line 510, in execute
    return self.cursor.execute(query, self._param_generator(params))
django.db.utils.DatabaseError: ORA-02261: such unique or primary key already exists in the table

tbicr · 2019-03-13T20:31:26Z

Is #11005 no longer a dependency of this?

Yep, removed skipIf for sqlite

Oracle errors:

Look like oracle doesn't have issue that solve this MR for another databases because it forbid same field constraints creation on database level.

So one of approaches can be to skip this tests for oracle.

Another side: for me oracle behavior looks logical and another way to solve issue can be forbid to create/change model with same fields constraints (if you didn't create it yet).

However this two approaches quite separate and can be done with different tickets, what do you think?

timgraham

I'm okay with skipping the test on Oracle.

timgraham · 2019-03-15T01:35:32Z

tests/schema/tests.py

+
+            # Drop the check constraint
+            with connection.schema_editor() as editor:
+                AuthorWithUniqueName._meta.constraints = []  # sqlite hack


I think we'd want more details about why this hack is needed.

Isn't the AuthorWithUniqueName an error? Shouldn't it be Author in this test? (The similar lines in the other tests use the model adjusted in that test.)

But if so, it's not causing a failure on SQLite, so what's it for?

added details

chaged AuthorWithUniqueName to Author, it doesn't raise because remove_constraint for sqlite use model._meta.constraints when recreate table and just had incorrect state in database after test run

timgraham · 2019-03-15T01:37:22Z

tests/schema/tests.py

+            # Ensure the constraints exist
+            constraints = self.get_constraints(Author._meta.db_table)
+            self.assertIn(custom_constraint_name, constraints)
+            self.assertEqual(


This would be more readable and consistent with our indentation style with something like:

foo_constraints = [ name for name, details in constraints.items() if details['columns'] == ['name'] and details['unique'] and name != custom_constraint_name] ] self.assertEqual(len(foo_constraints), 1)

(choosing a different name than "foo"

timgraham · 2019-03-15T01:38:15Z

tests/schema/tests.py

+        # Create table
+        with connection.schema_editor() as editor:
+            editor.create_model(AuthorWithUniqueName)
+


You can omit the blank line before each comment.

carltongibson · 2019-03-15T10:34:56Z

tests/schema/tests.py

+
+            # Drop the check constraint
+            with connection.schema_editor() as editor:
+                AuthorWithUniqueName._meta.constraints = []  # sqlite hack


Isn't the AuthorWithUniqueName an error? Shouldn't it be Author in this test? (The similar lines in the other tests use the model adjusted in that test.)

But if so, it's not causing a failure on SQLite, so what's it for?

carltongibson · 2019-03-15T10:40:03Z

tests/schema/tests.py

@@ -1671,6 +1731,60 @@ class Meta:
        with self.assertRaises(IntegrityError):
            Tag.objects.create(title='bar', slug='foo')

+    def test_remove_field_uniqueness_does_not_remove_custom_constraints(self):


test_removing_field_constraint_does_not_remove_meta_constraint? (Similar for other tests?)

sorry, look like I miss point of you questions, by the way all tests in PR pretty similar

carltongibson · 2019-03-15T10:41:34Z

tests/schema/tests.py

+        constraint = CheckConstraint(check=Q(height__gte=0), name='author_height_gte_0_check')
+        custom_constraint_name = constraint.name
+
+        try:


Question: the try...finally... is just to ensure test isolation yes?

yep, looks it unnecessary when state cleaned with last lines of test, so just removed it

django/db/backends/base/schema.py

tbicr · 2019-03-15T21:13:39Z

I'm okay with skipping the test on Oracle.

added skipIf for oracle

…t from removing Meta constraints.

…r from removing Meta constraints/indexes.

charettes · 2019-03-18T03:53:37Z

Thanks for hardwork here @tbicr and @timgraham.

tbicr force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from 9471a3a to c2a73ec Compare February 12, 2019 22:17

tbicr force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from c2a73ec to a728d46 Compare February 13, 2019 07:16

auvipy suggested changes Mar 10, 2019

View reviewed changes

tbicr force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from a728d46 to 23a85b1 Compare March 11, 2019 08:53

tbicr force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from 23a85b1 to d062194 Compare March 11, 2019 09:24

tbicr force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from d062194 to c07b4e6 Compare March 13, 2019 20:15

timgraham reviewed Mar 15, 2019

View reviewed changes

carltongibson reviewed Mar 15, 2019

View reviewed changes

felixxm reviewed Mar 15, 2019

View reviewed changes

django/db/backends/base/schema.py Outdated Show resolved Hide resolved

tbicr force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from c07b4e6 to d9be31f Compare March 15, 2019 21:13

timgraham force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from d9be31f to 293e8c0 Compare March 18, 2019 00:26

timgraham changed the title ~~Fixed #30172 -- meta index constraint conflict with same field constraints removing~~ Fixed #30172 -- Prevented Meta constraints from being removed in other migration operations. Mar 18, 2019

tbicr added 2 commits March 17, 2019 20:50

Refs #30172 -- Prevented removing a field's check or unique constrain…

4bb859e

…t from removing Meta constraints.

Refs #30172 -- Prevented removing a model Meta's index/unique_togethe…

5c17c27

…r from removing Meta constraints/indexes.

timgraham force-pushed the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch from 293e8c0 to 5c17c27 Compare March 18, 2019 00:51

timgraham merged commit 5c17c27 into django:master Mar 18, 2019

tbicr deleted the fix-meta-index-constraint-conflict-with-same-field-constraints-removing branch March 18, 2019 08:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #30172 -- Prevented Meta constraints from being removed in other migration operations. #10978

Fixed #30172 -- Prevented Meta constraints from being removed in other migration operations. #10978

tbicr commented Feb 11, 2019 •

edited by timgraham

Loading

tbicr commented Feb 12, 2019

auvipy left a comment

tbicr commented Mar 11, 2019

timgraham commented Mar 13, 2019

timgraham commented Mar 13, 2019

tbicr commented Mar 13, 2019

timgraham left a comment

timgraham Mar 15, 2019

carltongibson Mar 15, 2019 •

edited

Loading

tbicr Mar 15, 2019

timgraham Mar 15, 2019

tbicr Mar 15, 2019

timgraham Mar 15, 2019

tbicr Mar 15, 2019

carltongibson Mar 15, 2019 •

edited

Loading

carltongibson Mar 15, 2019

tbicr Mar 15, 2019

carltongibson Mar 15, 2019

tbicr Mar 15, 2019

tbicr commented Mar 15, 2019

charettes commented Mar 18, 2019

Fixed #30172 -- Prevented Meta constraints from being removed in other migration operations. #10978

Fixed #30172 -- Prevented Meta constraints from being removed in other migration operations. #10978

Conversation

tbicr commented Feb 11, 2019 • edited by timgraham Loading

tbicr commented Feb 12, 2019

auvipy left a comment

Choose a reason for hiding this comment

tbicr commented Mar 11, 2019

timgraham commented Mar 13, 2019

timgraham commented Mar 13, 2019

tbicr commented Mar 13, 2019

timgraham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carltongibson Mar 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carltongibson Mar 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbicr commented Mar 15, 2019

charettes commented Mar 18, 2019

tbicr commented Feb 11, 2019 •

edited by timgraham

Loading

carltongibson Mar 15, 2019 •

edited

Loading

carltongibson Mar 15, 2019 •

edited

Loading