Skip to content

Commit 28a0686

Browse files
mikiczDavid-Wobrock
authored andcommitted
Detect NULL default value as no default value.
This can happen when using `db_default=None` with `null=False` on a Django model field.
1 parent 8c8a0c6 commit 28a0686

9 files changed

Lines changed: 90 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ Feature:
44
- **Breaking change**: Handle custom Django app label when gathering migrations from git reference. (#262)
55
This means that, an app that was previously referenced by its folder name, will now be referenced by its Django app label.
66

7+
Bug:
8+
- Consider `DEFAULT NULL` as no default value being defined (#302)
9+
710
Miscellaneous:
811
- Add py.typed file (#303)
912
- Add support for Django 5.2 (#304)

src/django_migration_linter/sql_analyser/base.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ def has_not_null_column(sql_statements: list[str], **kwargs) -> bool:
4242
or sql.startswith("CREATE UNIQUE INDEX")
4343
):
4444
not_null_column = True
45-
if re.search("DEFAULT.*NOT NULL", sql):
45+
if re.search("DEFAULT (?!NULL).*NOT NULL", sql):
4646
has_default_value = True
47-
if "SET DEFAULT" in sql:
47+
if "SET DEFAULT" in sql and "SET DEFAULT NULL" not in sql:
4848
has_default_value = True
4949
if "DROP DEFAULT" in sql:
5050
has_default_value = False

tests/fixtures.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@
1111
ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DB_DEFAULT = (
1212
"app_add_not_null_column_followed_by_db_default"
1313
)
14+
ADD_NOT_NULL_COLUMN_WITH_NULL_DB_DEFAULT = (
15+
"app_add_not_null_column_with_null_db_default"
16+
)
1417
ALTER_COLUMN = "app_alter_column"
1518
ALTER_COLUMN_DROP_NOT_NULL = "app_alter_column_drop_not_null"
1619
DROP_UNIQUE_TOGETHER = "app_unique_together"

tests/functional/test_migration_linter.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import django
88
from django.conf import settings
99
from django.core.management import call_command
10+
from django.test import override_settings
1011

1112
from django_migration_linter import MigrationLinter
1213
from tests import fixtures
@@ -166,6 +167,15 @@ def test_accept_not_null_column_followed_by_adding_db_default(self):
166167
app = fixtures.ADD_NOT_NULL_COLUMN_FOLLOWED_BY_DB_DEFAULT
167168
self._test_linter_finds_errors(app)
168169

170+
@skipIf(django.VERSION[0] < 5, "db_default was implemented in Django 5.0")
171+
@override_settings(
172+
INSTALLED_APPS=settings.INSTALLED_APPS
173+
+ ["tests.test_project.app_add_not_null_column_with_null_db_default"]
174+
)
175+
def test_detected_not_null_column_with_null_db_default(self):
176+
app = fixtures.ADD_NOT_NULL_COLUMN_WITH_NULL_DB_DEFAULT
177+
self._test_linter_finds_errors(app)
178+
169179
def test_detect_make_column_not_null_with_lib_default(self):
170180
# The 'django-add-default-value' doesn't handle sqlite correctly
171181
app = fixtures.MAKE_NOT_NULL_WITH_LIB_DEFAULT
@@ -177,6 +187,9 @@ class MySqlBackwardCompatibilityDetectionTestCase(
177187
):
178188
databases = ["mysql"]
179189

190+
# test_detected_not_null_column_with_null_db_default is not included for mysql, because the setup raises
191+
# django.db.utils.OperationalError: (1067, "Invalid default value for 'not_null_field_db_default_null'")
192+
180193

181194
class PostgresqlBackwardCompatibilityDetectionTestCase(
182195
BaseBackwardCompatibilityDetection, unittest.TestCase
@@ -191,3 +204,12 @@ def test_create_index_exclusive(self):
191204
linter = self._launch_linter(fixtures.CREATE_INDEX_EXCLUSIVE)
192205
self.assertFalse(linter.has_errors)
193206
self.assertTrue(linter.nb_warnings)
207+
208+
@skipIf(django.VERSION[0] < 5, "db_default was implemented in Django 5.0")
209+
@override_settings(
210+
INSTALLED_APPS=settings.INSTALLED_APPS
211+
+ ["tests.test_project.app_add_not_null_column_with_null_db_default"]
212+
)
213+
def test_detected_not_null_column_with_null_db_default(self):
214+
app = fixtures.ADD_NOT_NULL_COLUMN_WITH_NULL_DB_DEFAULT
215+
self._test_linter_finds_errors(app)

tests/test_project/app_add_not_null_column_with_null_db_default/__init__.py

Whitespace-only changes.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Generated by Django 5.0.1 on 2024-02-04 20:07
2+
3+
from __future__ import annotations
4+
5+
from django.db import migrations, models
6+
7+
8+
class Migration(migrations.Migration):
9+
10+
initial = True
11+
12+
dependencies = []
13+
14+
operations = [
15+
migrations.CreateModel(
16+
name="A",
17+
fields=[
18+
(
19+
"id",
20+
models.AutoField(
21+
auto_created=True,
22+
primary_key=True,
23+
serialize=False,
24+
verbose_name="ID",
25+
),
26+
),
27+
("field", models.IntegerField()),
28+
],
29+
),
30+
]
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Generated by Django 5.0.1 on 2024-02-04 20:07
2+
3+
from __future__ import annotations
4+
5+
import django.db.models.functions.math
6+
from django.db import migrations, models
7+
8+
9+
class Migration(migrations.Migration):
10+
11+
dependencies = [
12+
("app_add_not_null_column_with_null_db_default", "0001_initial"),
13+
]
14+
15+
operations = [
16+
migrations.AddField(
17+
model_name="a",
18+
name="not_null_field_db_default_null",
19+
field=models.IntegerField(db_default=None, default=None, null=False),
20+
),
21+
]

tests/test_project/app_add_not_null_column_with_null_db_default/migrations/__init__.py

Whitespace-only changes.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
from __future__ import annotations
2+
3+
from django.db import models
4+
from django.db.models.functions import Pi
5+
6+
7+
class A(models.Model):
8+
field = models.IntegerField()
9+
not_null_field_db_default_null = models.IntegerField(null=False, db_default=None)

0 commit comments

Comments
 (0)