From 28a0686d3b4bf7b4b5e4e326bb59ff824fbf84a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikul=C3=A1=C5=A1=20Poul?= Date: Thu, 3 Apr 2025 16:46:13 +0100 Subject: [PATCH] Detect NULL default value as no default value. This can happen when using `db_default=None` with `null=False` on a Django model field. --- CHANGELOG.md | 3 ++ .../sql_analyser/base.py | 4 +-- tests/fixtures.py | 3 ++ tests/functional/test_migration_linter.py | 22 ++++++++++++++ .../__init__.py | 0 .../migrations/0001_initial.py | 30 +++++++++++++++++++ ...2_a_not_null_field_with_null_db_default.py | 21 +++++++++++++ .../migrations/__init__.py | 0 .../models.py | 9 ++++++ 9 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 tests/test_project/app_add_not_null_column_with_null_db_default/__init__.py create mode 100644 tests/test_project/app_add_not_null_column_with_null_db_default/migrations/0001_initial.py create mode 100644 tests/test_project/app_add_not_null_column_with_null_db_default/migrations/0002_a_not_null_field_with_null_db_default.py create mode 100644 tests/test_project/app_add_not_null_column_with_null_db_default/migrations/__init__.py create mode 100644 tests/test_project/app_add_not_null_column_with_null_db_default/models.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a60609..36f37ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ Feature: - **Breaking change**: Handle custom Django app label when gathering migrations from git reference. (#262) This means that, an app that was previously referenced by its folder name, will now be referenced by its Django app label. +Bug: +- Consider `DEFAULT NULL` as no default value being defined (#302) + Miscellaneous: - Add py.typed file (#303) - Add support for Django 5.2 (#304) diff --git a/src/django_migration_linter/sql_analyser/base.py b/src/django_migration_linter/sql_analyser/base.py index c27bfe9..2ad34bb 100644 --- a/src/django_migration_linter/sql_analyser/base.py +++ b/src/django_migration_linter/sql_analyser/base.py @@ -42,9 +42,9 @@ def has_not_null_column(sql_statements: list[str], **kwargs) -> bool: or sql.startswith("CREATE UNIQUE INDEX") ): not_null_column = True - if re.search("DEFAULT.*NOT NULL", sql): + if re.search("DEFAULT (?!NULL).*NOT NULL", sql): has_default_value = True - if "SET DEFAULT" in sql: + if "SET DEFAULT" in sql and "SET DEFAULT NULL" not in sql: has_default_value = True if "DROP DEFAULT" in sql: has_default_value = False diff --git a/tests/fixtures.py b/tests/fixtures.py index 77f5801..47fa4b2 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -11,6 +11,9 @@ ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DEFAULT = "app_add_not_null_column_followed_by_d ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DB_DEFAULT = ( "app_add_not_null_column_followed_by_db_default" ) +ADD_NOT_NULL_COLUMN_WITH_NULL_DB_DEFAULT = ( + "app_add_not_null_column_with_null_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 3d1fa5e..ab3edaf 100644 --- a/tests/functional/test_migration_linter.py +++ b/tests/functional/test_migration_linter.py @@ -7,6 +7,7 @@ from unittest import skipIf import django from django.conf import settings from django.core.management import call_command +from django.test import override_settings from django_migration_linter import MigrationLinter from tests import fixtures @@ -166,6 +167,15 @@ class SqliteBackwardCompatibilityDetectionTestCase( app = fixtures.ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DB_DEFAULT self._test_linter_finds_errors(app) + @skipIf(django.VERSION[0] < 5, "db_default was implemented in Django 5.0") + @override_settings( + INSTALLED_APPS=settings.INSTALLED_APPS + + ["tests.test_project.app_add_not_null_column_with_null_db_default"] + ) + def test_detected_not_null_column_with_null_db_default(self): + app = fixtures.ADD_NOT_NULL_COLUMN_WITH_NULL_DB_DEFAULT + self._test_linter_finds_errors(app) + def test_detect_make_column_not_null_with_lib_default(self): # The 'django-add-default-value' doesn't handle sqlite correctly app = fixtures.MAKE_NOT_NULL_WITH_LIB_DEFAULT @@ -177,6 +187,9 @@ class MySqlBackwardCompatibilityDetectionTestCase( ): databases = ["mysql"] + # test_detected_not_null_column_with_null_db_default is not included for mysql, because the setup raises + # django.db.utils.OperationalError: (1067, "Invalid default value for 'not_null_field_db_default_null'") + class PostgresqlBackwardCompatibilityDetectionTestCase( BaseBackwardCompatibilityDetection, unittest.TestCase @@ -191,3 +204,12 @@ class PostgresqlBackwardCompatibilityDetectionTestCase( linter = self._launch_linter(fixtures.CREATE_INDEX_EXCLUSIVE) self.assertFalse(linter.has_errors) self.assertTrue(linter.nb_warnings) + + @skipIf(django.VERSION[0] < 5, "db_default was implemented in Django 5.0") + @override_settings( + INSTALLED_APPS=settings.INSTALLED_APPS + + ["tests.test_project.app_add_not_null_column_with_null_db_default"] + ) + def test_detected_not_null_column_with_null_db_default(self): + app = fixtures.ADD_NOT_NULL_COLUMN_WITH_NULL_DB_DEFAULT + self._test_linter_finds_errors(app) diff --git a/tests/test_project/app_add_not_null_column_with_null_db_default/__init__.py b/tests/test_project/app_add_not_null_column_with_null_db_default/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_project/app_add_not_null_column_with_null_db_default/migrations/0001_initial.py b/tests/test_project/app_add_not_null_column_with_null_db_default/migrations/0001_initial.py new file mode 100644 index 0000000..9c5bed6 --- /dev/null +++ b/tests/test_project/app_add_not_null_column_with_null_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_with_null_db_default/migrations/0002_a_not_null_field_with_null_db_default.py b/tests/test_project/app_add_not_null_column_with_null_db_default/migrations/0002_a_not_null_field_with_null_db_default.py new file mode 100644 index 0000000..412464c --- /dev/null +++ b/tests/test_project/app_add_not_null_column_with_null_db_default/migrations/0002_a_not_null_field_with_null_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_with_null_db_default", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="a", + name="not_null_field_db_default_null", + field=models.IntegerField(db_default=None, default=None, null=False), + ), + ] diff --git a/tests/test_project/app_add_not_null_column_with_null_db_default/migrations/__init__.py b/tests/test_project/app_add_not_null_column_with_null_db_default/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_project/app_add_not_null_column_with_null_db_default/models.py b/tests/test_project/app_add_not_null_column_with_null_db_default/models.py new file mode 100644 index 0000000..70f9a21 --- /dev/null +++ b/tests/test_project/app_add_not_null_column_with_null_db_default/models.py @@ -0,0 +1,9 @@ +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_null = models.IntegerField(null=False, db_default=None)