Merge pull request #84 from kraken-tech/s-cooper18/fix/more-useful-constraint-deletion-output

Bugfix: drop unique constraint sqlmigrate is noop

Author: s-cooper18

CHANGELOG.md

@@ -8,7 +8,13 @@ Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to
 
 ## [Unreleased]
 
-_No notable unreleased changes_
+### Fixed
+
+- `SaferRemoveUniqueConstraint` now produces the best-effort plan for a migration that would be
+  performed if the constraint wanting to be removed does in fact exist when running `sqlmigrate`.
+  Previously, this would act as if the constraint did not exist is using `sqlmigrate`.
+- `SaferAddUniqueConstraint` now produces a backwards plan as if the constraint has already been
+   created when running `sqlmigrate`.
 
 ## [0.1.19] - 2025-04-04
 

src/django_pg_migration_tools/operations.py

@@ -506,7 +506,7 @@ def drop_unique_constraint(
             )
             return
 
-        if not self._constraint_exists(schema_editor, constraint):
+        if not self._constraint_exists(schema_editor, constraint, collect_default=True):
             # Nothing to delete.
             return
 
@@ -591,7 +591,9 @@ def _can_create_constraint(
         constraint: models.UniqueConstraint,
         raise_if_exists: bool,
     ) -> bool:
-        constraint_exists = self._constraint_exists(schema_editor, constraint)
+        constraint_exists = self._constraint_exists(
+            schema_editor, constraint, collect_default=False
+        )
         if raise_if_exists and constraint_exists:
             raise ConstraintAlreadyExists(
                 f"Cannot create a constraint with the name "
@@ -619,7 +621,7 @@ def _constraint_exists(
         self,
         schema_editor: base_schema.BaseDatabaseSchemaEditor,
         constraint: models.UniqueConstraint | models.CheckConstraint,
-        collect_default: bool = False,
+        collect_default: bool,
     ) -> bool:
         return _run_introspection_query(
             schema_editor,

tests/django_pg_migration_tools/test_operations.py

@@ -910,6 +910,41 @@ def test_when_collecting_only(self):
             == 'ALTER TABLE "example_app_intmodel" ADD CONSTRAINT "unique_int_field" UNIQUE USING INDEX "unique_int_field";'
         )
 
+    @pytest.mark.django_db(transaction=True)
+    def test_when_collecting_only_reversed(self):
+        # Prove that the constraint exists before the operation removes it.
+        with connection.cursor() as cursor:
+            cursor.execute(
+                psycopg_sql.SQL(operations.ConstraintQueries.CHECK_EXISTING_CONSTRAINT)
+                .format(constraint_name=psycopg_sql.Literal("unique_char_field"))
+                .as_string(cursor.connection)
+            )
+            assert cursor.fetchone()
+
+        project_state = ProjectState()
+        project_state.add_model(ModelState.from_model(CharModel))
+        new_state = project_state.clone()
+
+        operation = operations.SaferAddUniqueConstraint(
+            model_name="charmodel",
+            constraint=UniqueConstraint(
+                fields=("char_field",),
+                name="unique_char_field",
+            ),
+        )
+
+        with connection.schema_editor(atomic=False, collect_sql=True) as editor:
+            with utils.CaptureQueriesContext(connection) as queries:
+                operation.database_backwards(
+                    self.app_label, editor, from_state=project_state, to_state=new_state
+                )
+
+        assert len(editor.collected_sql) == 1
+        assert len(queries) == 0
+        assert editor.collected_sql[0] == (
+            'ALTER TABLE "example_app_charmodel" DROP CONSTRAINT "unique_char_field";'
+        )
+
     # Disable the overall test transaction because a unique concurrent index
     # creation followed by a constraint addition cannot be triggered/tested
     # inside of a transaction.
@@ -1576,6 +1611,38 @@ def test_when_not_allowed_to_migrate_by_the_router(self):
         # the router.
         assert len(queries) == 0
 
+    @pytest.mark.django_db(transaction=True)
+    def test_when_collecting_only(self):
+        # Prove that the constraint exists before the operation removes it.
+        with connection.cursor() as cursor:
+            cursor.execute(
+                psycopg_sql.SQL(operations.ConstraintQueries.CHECK_EXISTING_CONSTRAINT)
+                .format(constraint_name=psycopg_sql.Literal("unique_char_field"))
+                .as_string(cursor.connection)
+            )
+            assert cursor.fetchone()
+
+        project_state = ProjectState()
+        project_state.add_model(ModelState.from_model(CharModel))
+        new_state = project_state.clone()
+
+        operation = operations.SaferRemoveUniqueConstraint(
+            model_name="charmodel",
+            name="unique_char_field",
+        )
+
+        with connection.schema_editor(atomic=False, collect_sql=True) as editor:
+            with utils.CaptureQueriesContext(connection) as queries:
+                operation.database_forwards(
+                    self.app_label, editor, from_state=project_state, to_state=new_state
+                )
+
+        assert len(editor.collected_sql) == 1
+        assert len(queries) == 0
+        assert editor.collected_sql[0] == (
+            'ALTER TABLE "example_app_charmodel" DROP CONSTRAINT "unique_char_field";'
+        )
+
     @pytest.mark.django_db(transaction=True)
     def test_does_nothing_if_constraint_does_not_exist(self):
         # Remove the constraint so that the migration becomes a noop.