Skip to content

Commit

Permalink
Support Django 5.0 db_default attribute.
Browse files Browse the repository at this point in the history
A DB default is not considered backward incompatible, as the default value is kept on DB-level.
See https://docs.djangoproject.com/en/dev/releases/5.0/#database-computed-default-values
  • Loading branch information
David-Wobrock committed Mar 29, 2024
1 parent 3a557cb commit 18a39cb
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
3 changes: 2 additions & 1 deletion docs/incompatibilities.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand All @@ -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/)

Expand Down
26 changes: 14 additions & 12 deletions src/django_migration_linter/sql_analyser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("(?<!DROP )(?<!IS )NOT NULL", sql) and not (
sql.startswith("CREATE TABLE") or sql.startswith("CREATE INDEX")
):
not_null_column = True
if re.search("DEFAULT.*NOT NULL", sql):
has_default_value = True
if "SET DEFAULT" in sql:
ends_with_default = True
elif "DROP DEFAULT" in sql:
ends_with_default = False
return (
any(
re.search("(?<!DROP )(?<!IS )NOT NULL", sql)
and not (sql.startswith("CREATE TABLE") or sql.startswith("CREATE INDEX"))
for sql in sql_statements
)
and ends_with_default is False
)
has_default_value = True
if "DROP DEFAULT" in sql:
has_default_value = False

return not_null_column and not has_default_value


def has_add_unique(sql_statements: list[str], **kwargs) -> bool:
Expand Down
3 changes: 3 additions & 0 deletions tests/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
12 changes: 12 additions & 0 deletions tests/functional/test_migration_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import os
import unittest
from unittest import skipIf

import django
from django.conf import settings
from django.core.management import call_command

Expand Down Expand Up @@ -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, "db_default was implemented in Django 5.0")
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)
Expand Down Expand Up @@ -128,6 +135,11 @@ def test_detect_make_column_not_null_with_django_default(self):
app = fixtures.MAKE_NOT_NULL_WITH_DJANGO_DEFAULT
self._test_linter_finds_errors(app)

@skipIf(django.VERSION[0] < 5, "db_default was implemented in Django 5.0")
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_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
Expand Down
Empty file.
Original file line number Diff line number Diff line change
@@ -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()),
],
),
]
Original file line number Diff line number Diff line change
@@ -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()),
),
]
Empty file.
Original file line number Diff line number Diff line change
@@ -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(),
)
8 changes: 8 additions & 0 deletions tests/test_project/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)))

Expand Down Expand Up @@ -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",
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/test_sql_analyser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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;",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;',
Expand Down

0 comments on commit 18a39cb

Please sign in to comment.