diff --git a/README.md b/README.md index 8564797..b60582e 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,17 @@ class Migration(migrations.Migration): ``` +### Changing field type + +Changing the type of a field is generally not safe to do because it causes a +full table rewrite, during which the table will be fully locked. Additionally +old code still running after the migration has been applied might write +unsupported values to the column. + +Rather than changing the type of a column you should add a new column and +manually migrate data from the old column to the new one. + + ### Adding indexes Checks if the migration contains an `AddIndex` operation and suggests using diff --git a/migration_checker/checks.py b/migration_checker/checks.py index 4cf8bdf..3bcf974 100644 --- a/migration_checker/checks.py +++ b/migration_checker/checks.py @@ -7,6 +7,7 @@ AddConstraint, AddField, AddIndex, + AlterField, Migration, RemoveField, RenameField, @@ -16,13 +17,14 @@ ) from django.db.migrations.operations.fields import FieldOperation from django.db.migrations.operations.models import ModelOperation -from django.db.migrations.state import ProjectState +from django.db.migrations.state import ModelState, ProjectState from .warnings import ( ADD_INDEX_IN_SEPARATE_MIGRATION, ADDING_CONSTRAINT, ADDING_FIELD_WITH_CHECK, ADDING_NON_NULLABLE_FIELD, + ALTER_FIELD, ALTERING_MULTIPLE_MODELS, ATOMIC_DATA_MIGRATION, REMOVING_FIELD, @@ -166,6 +168,25 @@ def check_validate_constraint( yield VALIDATE_CONSTRAINT_SEPARATELY +def check_alter_field( + *, migration: Migration, state: ProjectState +) -> Iterable[Warning]: + def changes_type(operation: AlterField) -> bool: + # Extract the old field + model: ModelState = state.models[(migration.app_label, operation.model_name)] + field = model.fields[operation.name] + # TODO: Also check other things that have changed + return type(field) is not type(operation.field) # noqa: E721 + + alter_fields = [ + operation + for operation in migration.operations + if isinstance(operation, AlterField) + ] + if any(changes_type(operation) for operation in alter_fields): + yield ALTER_FIELD + + ALL_CHECKS: list[Check] = [ check_add_index, check_add_non_nullable_field, @@ -178,6 +199,7 @@ def check_validate_constraint( check_field_with_check_constraint, check_add_constraint, check_validate_constraint, + check_alter_field, ] diff --git a/migration_checker/warnings.py b/migration_checker/warnings.py index 06f2d90..06d8a94 100644 --- a/migration_checker/warnings.py +++ b/migration_checker/warnings.py @@ -162,3 +162,13 @@ def __repr__(self) -> str: "constraint validation operations in a separate migration." ), ) + +ALTER_FIELD = Warning( + level=Level.DANGER, + title="Changing column type", + description=( + "This migration is changing the type of a column, which is generally " + "not safe because it fully locks the table and can cause issues with " + "writes from old code." + ), +) diff --git a/tests/test_checks.py b/tests/test_checks.py index 6a5d0d6..58ac6c5 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -5,6 +5,7 @@ AddConstraint, AddField, AddIndex, + AlterField, RemoveField, RenameField, RenameModel, @@ -14,6 +15,7 @@ from django.db.migrations.state import ProjectState from django.db.models import ( AutoField, + BigAutoField, CheckConstraint, Index, IntegerField, @@ -27,6 +29,7 @@ ADDING_CONSTRAINT, ADDING_FIELD_WITH_CHECK, ADDING_NON_NULLABLE_FIELD, + ALTER_FIELD, ALTERING_MULTIPLE_MODELS, REMOVING_FIELD, RENAMING_FIELD, @@ -131,6 +134,16 @@ def test_add_constraint() -> None: assert check_migration(operation) == {ADDING_CONSTRAINT} +def test_alter_field() -> None: + operation = AlterField(model_name="foo", name="id", field=BigAutoField()) + assert check_migration(operation) == {ALTER_FIELD} + + +def test_alter_field_to_nullable() -> None: + operation = AlterField(model_name="foo", name="id", field=AutoField(null=True)) + assert check_migration(operation) == set() + + @pytest.mark.skipif(django.VERSION < (4, 0), reason="Not supported in Django < 4.0") def test_add_field_and_validate_constraint() -> None: from django.contrib.postgres.operations import ValidateConstraint