Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safer remove field foreign key #69

Merged
merged 6 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
68 changes: 68 additions & 0 deletions docs/usage/operations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
58 changes: 56 additions & 2 deletions src/django_pg_migration_tools/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading