diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b5ef18..89cc73a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ ## 5.x.y +Feature: +- Support Django 5.0 `db_default` attribute (issue #275) + Bug: - Don't detect 'IS NOT NULL' as backward incompatible changes (issue #263) diff --git a/docs/incompatibilities.md b/docs/incompatibilities.md index 90a5bf0..873480d 100644 --- a/docs/incompatibilities.md +++ b/docs/incompatibilities.md @@ -71,7 +71,7 @@ Only rolling back the code will make all new insertions crash because Django doe One would think that adding a default value in Django will prevent these errors. A common misconception is that the Django default value is translated to a database default. -But Django actually uses the default value to fill new new column on existing rows and to set an unspecified column value to its default. +But Django actually uses the default value to fill the new column on existing rows and to set an unspecified column value to its default. The latter is done at the application level, by Django and not by the database because the default value was dropped during migration. You can read more about this in the [Django and its default values blog post](https://medium.com/botify-labs/django-and-its-default-values-c21a13cff9f). @@ -90,6 +90,7 @@ You can read more about this in the [Django and its default values blog post](ht :white_check_mark: **Solutions**: - Make the column nullable +- Set a database default using the Django 5.0 attribute `db_default`. See the [Django docs](https://docs.djangoproject.com/en/dev/releases/5.0/#database-computed-default-values) - Set a database default using Django's [RunSQL](https://docs.djangoproject.com/en/dev/ref/migration-operations/#django.db.migrations.operations.RunSQL) - Set a database default using [django-add-default-value](https://github.com/3YOURMIND/django-add-default-value/) diff --git a/src/django_migration_linter/sql_analyser/base.py b/src/django_migration_linter/sql_analyser/base.py index 0dcd9b6..bc037f8 100644 --- a/src/django_migration_linter/sql_analyser/base.py +++ b/src/django_migration_linter/sql_analyser/base.py @@ -32,20 +32,22 @@ def update_migration_checks( def has_not_null_column(sql_statements: list[str], **kwargs) -> bool: # TODO: improve to detect that the same column is concerned - ends_with_default = False + not_null_column = False + has_default_value = False + for sql in sql_statements: + if re.search("(? bool: diff --git a/tests/fixtures.py b/tests/fixtures.py index 38224c4..5432c0d 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -8,6 +8,9 @@ RENAME_TABLE = "app_rename_table" IGNORE_MIGRATION = "app_ignore_migration" ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DEFAULT = "app_add_not_null_column_followed_by_default" +ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DB_DEFAULT = ( + "app_add_not_null_column_followed_by_db_default" +) ALTER_COLUMN = "app_alter_column" ALTER_COLUMN_DROP_NOT_NULL = "app_alter_column_drop_not_null" DROP_UNIQUE_TOGETHER = "app_unique_together" diff --git a/tests/functional/test_migration_linter.py b/tests/functional/test_migration_linter.py index c2b1549..675e8f5 100644 --- a/tests/functional/test_migration_linter.py +++ b/tests/functional/test_migration_linter.py @@ -2,7 +2,9 @@ import os import unittest +from unittest import skip, skipIf +import django from django.conf import settings from django.core.management import call_command @@ -88,6 +90,11 @@ def test_accept_not_null_column_followed_by_adding_default(self): app = fixtures.ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DEFAULT self._test_linter_finds_no_errors(app) + @skipIf(django.VERSION[0] < 5) + def test_accept_not_null_column_followed_by_adding_db_default(self): + app = fixtures.ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DB_DEFAULT + self._test_linter_finds_no_errors(app) + def test_detect_alter_column(self): app = fixtures.ALTER_COLUMN self._test_linter_finds_no_errors(app) diff --git a/tests/test_project/app_add_not_null_column_followed_by_db_default/__init__.py b/tests/test_project/app_add_not_null_column_followed_by_db_default/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/0001_initial.py b/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/0001_initial.py new file mode 100644 index 0000000..9c5bed6 --- /dev/null +++ b/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/0001_initial.py @@ -0,0 +1,30 @@ +# Generated by Django 5.0.1 on 2024-02-04 20:07 + +from __future__ import annotations + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [] + + operations = [ + migrations.CreateModel( + name="A", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("field", models.IntegerField()), + ], + ), + ] diff --git a/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/0002_a_not_null_field_db_default.py b/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/0002_a_not_null_field_db_default.py new file mode 100644 index 0000000..5bd6e3b --- /dev/null +++ b/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/0002_a_not_null_field_db_default.py @@ -0,0 +1,21 @@ +# Generated by Django 5.0.1 on 2024-02-04 20:07 + +from __future__ import annotations + +import django.db.models.functions.math +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("app_add_not_null_column_followed_by_db_default", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="a", + name="not_null_field_db_default", + field=models.IntegerField(db_default=django.db.models.functions.math.Pi()), + ), + ] diff --git a/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/__init__.py b/tests/test_project/app_add_not_null_column_followed_by_db_default/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_project/app_add_not_null_column_followed_by_db_default/models.py b/tests/test_project/app_add_not_null_column_followed_by_db_default/models.py new file mode 100644 index 0000000..1f32388 --- /dev/null +++ b/tests/test_project/app_add_not_null_column_followed_by_db_default/models.py @@ -0,0 +1,12 @@ +from __future__ import annotations + +from django.db import models +from django.db.models.functions import Pi + + +class A(models.Model): + field = models.IntegerField() + not_null_field_db_default = models.IntegerField( + null=False, + db_default=Pi(), + ) diff --git a/tests/test_project/app_add_not_null_column_followed_by_default/models.py b/tests/test_project/app_add_not_null_column_followed_by_default/models.py index cfb119c..629479b 100644 --- a/tests/test_project/app_add_not_null_column_followed_by_default/models.py +++ b/tests/test_project/app_add_not_null_column_followed_by_default/models.py @@ -1,6 +1,7 @@ from __future__ import annotations from django.db import models +from django.db.models.functions import Pi class A(models.Model): diff --git a/tests/test_project/settings.py b/tests/test_project/settings.py index b23edb2..d71663d 100644 --- a/tests/test_project/settings.py +++ b/tests/test_project/settings.py @@ -14,6 +14,8 @@ import os +import django + # Build paths inside the project like this: os.path.join(BASE_DIR, ...) BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -59,6 +61,12 @@ "tests.test_project.app_create_index_exclusive", ] +if django.VERSION[0] >= 5: + # db_default attribute was only added in Django 5.0 + INSTALLED_APPS.append( + "tests.test_project.app_add_not_null_column_followed_by_db_default" + ) + MIDDLEWARE = [ "django.middleware.security.SecurityMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", diff --git a/tests/unit/test_sql_analyser.py b/tests/unit/test_sql_analyser.py index 48daf0d..0e903c0 100644 --- a/tests/unit/test_sql_analyser.py +++ b/tests/unit/test_sql_analyser.py @@ -66,6 +66,15 @@ def test_add_not_null(self): ] self.assertBackwardIncompatibleSql(sql) + def test_add_not_null_without_dropping_default(self): + sql = ( + "ALTER TABLE `app_add_not_null_column_a` ADD COLUMN `new_not_null_field` integer DEFAULT 1 NOT NULL;", + ) + self.assertValidSql(sql) + + sql = "ALTER TABLE `app_add_not_null_column_followed_by_default_a` ADD COLUMN `not_null_field_db_default` integer DEFAULT (PI()) NOT NULL;" + self.assertValidSql(sql) + def test_add_not_null_followed_by_default(self): sql = [ "ALTER TABLE `app_add_not_null_column_followed_by_default_a` ADD COLUMN `new_not_null_field` integer DEFAULT 1 NOT NULL;", @@ -147,6 +156,15 @@ def test_add_not_null(self): ] self.assertBackwardIncompatibleSql(sql) + def test_add_not_null_without_dropping_default(self): + sql = [ + 'CREATE TABLE "new__app_add_not_null_column_followed_by_default_a" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "not_null_field_db_default" integer DEFAULT (PI()) NOT NULL, "field" integer NOT NULL, "not_null_field" integer NOT NULL);', + 'INSERT INTO "new__app_add_not_null_column_followed_by_default_a" ("id", "field", "not_null_field") SELECT "id", "field", "not_null_field" FROM "app_add_not_null_column_followed_by_default_a";', + 'DROP TABLE "app_add_not_null_column_followed_by_default_a";', + 'ALTER TABLE "new__app_add_not_null_column_followed_by_default_a" RENAME TO "app_add_not_null_column_followed_by_default_a";', + ] + self.assertBackwardIncompatibleSql(sql) + def test_create_table_with_not_null(self): sql = 'CREATE TABLE "app_create_table_with_not_null_column_a" ("id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, "field" varchar(150) NOT NULL);' self.assertValidSql(sql) @@ -207,6 +225,10 @@ def test_alter_column(self): sql = 'ALTER TABLE "app_alter_column_a" ALTER COLUMN "field" TYPE varchar(10) USING "field"::varchar(10);' self.assertBackwardIncompatibleSql(sql) + def test_add_not_null_without_dropping_default(self): + sql = 'ALTER TABLE "app_add_not_null_column_followed_by_default_a" ADD COLUMN "not_null_field_db_default" integer DEFAULT (PI()) NOT NULL;' + self.assertValidSql(sql) + def test_not_null_followed_by_default(self): sql = [ 'ALTER TABLE "app_add_not_null_column_followed_by_default_a" ADD COLUMN "not_null_field" integer DEFAULT 1 NOT NULL;',