Merge pull request #69 from kraken-tech/safer-remove-field-foreign-key

Safer remove field foreign key

CHANGELOG.md

@@ -12,6 +12,7 @@ Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to
 
 - Enhanced `SaferAddUniqueConstraint` to support a `UniqueConstraint` with the
   `deferrable` argument.
+- A new operation to remove a foreign key field: `SaferRemoveFieldForeignKey`.
 
 ## [0.1.16] - 2025-01-08
 

docs/usage/operations.rst

@@ -517,6 +517,7 @@ Class Definitions
               ),
           ]
 
+.. _safer_add_field_foreign_key:
 .. py:class:: SaferAddFieldForeignKey(model_name: str, name: str, field: models.ForeignKey)
 
     Provides a safer way to add a foreign key field to an existing model
@@ -636,6 +637,73 @@ Class Definitions
               ),
           ]
 
+.. py:class:: SaferRemoveFieldForeignKey(model_name: str, name: str)
+
+    Provides a safer way to remove a foreign key field.
+
+    :param model_name: Model name in lowercase without underscores.
+    :type model_name: str
+    :param name: The column name for the foreign key field to be deleted.
+    :type name: str
+
+    **Why use this SaferRemoveFieldForeignKey operation?**
+    ------------------------------------------------------
+
+    The operation that Django provides (``RemoveField``) has the
+    following limitations:
+
+    1. The operation fails if the field has already been removed (not
+       idempotent).
+    2. When reverting, the alter table statement provided by Django to recreate
+       the foreign key will block reads and writes on the table.
+
+    This custom operation fixes those problems by:
+
+    - Having a custom forward operation that will only attempt to drop the
+      foreign key field if the field exists.
+    - Having a custom backward operation that will add the foreign key back
+      without blocking any reads/writes. This is achieved through the same
+      strategy of :ref:`SaferAddFieldForeignKey <safer_add_field_foreign_key>`.
+
+    How to use
+    ----------
+
+    1. Remove the ForeignKey field from your model:
+
+    .. code-block:: diff
+
+      -    bar = models.ForeignKey(Bar, null=True, on_delete=models.CASCADE)
+
+    2. Make the new migration:
+
+    .. code-block:: bash
+
+      ./manage.py makemigrations
+
+    3. The only changes you need to perform are:
+
+       1. Swap Django's ``RemoveField`` for this package's
+          ``SaferRemoveFieldForeignKey`` operation.
+       2. Use a non-atomic migration.
+
+    .. code-block:: diff
+
+      + from django_pg_migration_tools import operations
+      from django.db import migrations
+
+
+      class Migration(migrations.Migration):
+      +   atomic = False
+
+          dependencies = [("myapp", "0042_dependency")]
+
+          operations = [
+      -        migrations.RemoveField(
+      +        operations.SaferRemoveFieldForeignKey(
+                  model_name="mymodel",
+                  name="bar",
+              ),
+          ]
 
 .. py:class:: SaferAddCheckConstraint(model_name: str, constraint: models.CheckConstraint)
 

src/django_pg_migration_tools/operations.py

@@ -1262,12 +1262,12 @@ def drop_fk_field(self) -> None:
         ):
             return
 
-        if not self._column_exists():
+        if not self._column_exists(collect_default=True):
             return
 
         self._alter_table_drop_column()
 
-    def _column_exists(self) -> bool:
+    def _column_exists(self, collect_default: bool = False) -> bool:
         return _run_introspection_query(
             self.schema_editor,
             psycopg_sql.SQL(ColumnQueries.CHECK_COLUMN_EXISTS)
@@ -1276,6 +1276,7 @@ def _column_exists(self) -> bool:
                 column_name=psycopg_sql.Literal(self.column_name),
             )
             .as_string(self.schema_editor.connection.connection),
+            collect_default=collect_default,
         )
 
     def _get_remote_model(self) -> models.Model:
@@ -1455,6 +1456,59 @@ def describe(self) -> str:
         )
 
 
+class SaferRemoveFieldForeignKey(operation_fields.RemoveField):
+    def database_forwards(
+        self,
+        app_label: str,
+        schema_editor: base_schema.BaseDatabaseSchemaEditor,
+        from_state: migrations.state.ProjectState,
+        to_state: migrations.state.ProjectState,
+    ) -> None:
+        field = from_state.apps.get_model(app_label, self.model_name)._meta.get_field(
+            self.name
+        )
+        ForeignKeyManager(
+            app_label,
+            schema_editor,
+            from_state=from_state,
+            to_state=to_state,
+            model=to_state.apps.get_model(app_label, self.model_name),
+            model_name=self.model_name,
+            column_name=self.name,
+            field=field,
+            unique=False,
+        ).drop_fk_field()
+
+    def database_backwards(
+        self,
+        app_label: str,
+        schema_editor: base_schema.BaseDatabaseSchemaEditor,
+        from_state: migrations.state.ProjectState,
+        to_state: migrations.state.ProjectState,
+    ) -> None:
+        field = to_state.apps.get_model(app_label, self.model_name)._meta.get_field(
+            self.name
+        )
+        ForeignKeyManager(
+            app_label,
+            schema_editor,
+            from_state=from_state,
+            to_state=to_state,
+            model=to_state.apps.get_model(app_label, self.model_name),
+            model_name=self.model_name,
+            column_name=self.name,
+            field=field,
+            unique=False,
+        ).add_fk_field()
+
+    def describe(self) -> str:
+        base = super().describe()
+        return (
+            f"{base}. Note: Using django_pg_migration_tools "
+            f"SaferRemoveFieldForeignKey operation."
+        )
+
+
 class SaferAddFieldOneToOne(operation_fields.AddField):
     """
     Django's OneToOneField behaves the same way as a ForeignKey. Except that