Skip to content

Commit

Permalink
Fixed #35359 -- Fixed migration operations ordering when adding field…
Browse files Browse the repository at this point in the history
…s referenced by GeneratedField.expression.

Thank you to Simon Charette for the review.
  • Loading branch information
DevilsAutumn authored and sarahboyce committed May 2, 2024
1 parent 97d48cd commit 41f26da
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 1 deletion.
23 changes: 23 additions & 0 deletions django/db/migrations/autodetector.py
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,8 @@ def _generate_added_field(self, app_label, model_name, field_name):
self.to_state,
)
)
if field.generated:
dependencies.extend(self._get_dependencies_for_generated_field(field))
# You can't just add NOT NULL fields with no default or fields
# which don't allow empty strings as default.
time_fields = (models.DateField, models.DateTimeField, models.TimeField)
Expand Down Expand Up @@ -1547,6 +1549,27 @@ def _get_dependencies_for_foreign_key(app_label, model_name, field, project_stat
)
return dependencies

def _get_dependencies_for_generated_field(self, field):
dependencies = []
referenced_base_fields = models.Q(field.expression).referenced_base_fields
newly_added_fields = sorted(self.new_field_keys - self.old_field_keys)
for app_label, model_name, added_field_name in newly_added_fields:
added_field = self.to_state.models[app_label, model_name].get_field(
added_field_name
)
if (
added_field.remote_field and added_field.remote_field.model
) or added_field.name in referenced_base_fields:
dependencies.append(
OperationDependency(
app_label,
model_name,
added_field.name,
OperationDependency.Type.CREATE,
)
)
return dependencies

def _get_dependencies_for_model(self, app_label, model_name):
"""Return foreign key dependencies of the given model."""
dependencies = []
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/5.0.5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,7 @@ Bugfixes

* Allowed importing ``aprefetch_related_objects`` from ``django.db.models``
(:ticket:`35392`).

* Fixed a bug in Django 5.0 that caused a migration crash when a field which
is referenced by ``GeneratedField.expression`` was added after the
``GeneratedField`` operation (:ticket:`35359`).
77 changes: 77 additions & 0 deletions tests/migrations/test_autodetector.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.db.migrations.loader import MigrationLoader
from django.db.migrations.questioner import MigrationQuestioner
from django.db.migrations.state import ModelState, ProjectState
from django.db.models.functions import Concat, Lower
from django.test import SimpleTestCase, TestCase, override_settings
from django.test.utils import isolate_lru_cache

Expand Down Expand Up @@ -1369,6 +1370,82 @@ def test_add_date_fields_with_auto_now_add_asking_for_default(
self.assertOperationFieldAttributes(changes, "testapp", 0, 2, auto_now_add=True)
self.assertEqual(mocked_ask_method.call_count, 3)

def test_add_field_before_generated_field(self):
initial_state = ModelState(
"testapp",
"Author",
[
("name", models.CharField(max_length=20)),
],
)
updated_state = ModelState(
"testapp",
"Author",
[
("name", models.CharField(max_length=20)),
("surname", models.CharField(max_length=20)),
(
"lower_full_name",
models.GeneratedField(
expression=Concat(Lower("name"), Lower("surname")),
output_field=models.CharField(max_length=30),
db_persist=True,
),
),
],
)
changes = self.get_changes([initial_state], [updated_state])
self.assertNumberMigrations(changes, "testapp", 1)
self.assertOperationTypes(changes, "testapp", 0, ["AddField", "AddField"])
self.assertOperationFieldAttributes(
changes, "testapp", 0, 1, expression=Concat(Lower("name"), Lower("surname"))
)

def test_add_fk_before_generated_field(self):
initial_state = ModelState(
"testapp",
"Author",
[
("name", models.CharField(max_length=20)),
],
)
updated_state = [
ModelState(
"testapp",
"Publisher",
[
("name", models.CharField(max_length=20)),
],
),
ModelState(
"testapp",
"Author",
[
("name", models.CharField(max_length=20)),
(
"publisher",
models.ForeignKey("testapp.Publisher", models.CASCADE),
),
(
"lower_full_name",
models.GeneratedField(
expression=Concat("name", "publisher_id"),
output_field=models.CharField(max_length=20),
db_persist=True,
),
),
],
),
]
changes = self.get_changes([initial_state], updated_state)
self.assertNumberMigrations(changes, "testapp", 1)
self.assertOperationTypes(
changes, "testapp", 0, ["CreateModel", "AddField", "AddField"]
)
self.assertOperationFieldAttributes(
changes, "testapp", 0, 2, expression=Concat("name", "publisher_id")
)

def test_remove_field(self):
"""Tests autodetection of removed fields."""
changes = self.get_changes([self.author_name], [self.author_empty])
Expand Down
50 changes: 49 additions & 1 deletion tests/migrations/test_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from django.db.migrations.state import ModelState, ProjectState
from django.db.models import F
from django.db.models.expressions import Value
from django.db.models.functions import Abs, Pi
from django.db.models.functions import Abs, Concat, Pi
from django.db.transaction import atomic
from django.test import (
SimpleTestCase,
Expand Down Expand Up @@ -1379,6 +1379,54 @@ def test_add_field(self):
self.assertEqual(definition[1], [])
self.assertEqual(sorted(definition[2]), ["field", "model_name", "name"])

@skipUnlessDBFeature("supports_stored_generated_columns")
def test_add_generate_field(self):
app_label = "test_adgf"
project_state = self.apply_operations(
app_label,
ProjectState(),
operations=[
migrations.CreateModel(
"Rider",
fields=[
("id", models.AutoField(primary_key=True)),
],
),
migrations.CreateModel(
"Pony",
fields=[
("id", models.AutoField(primary_key=True)),
("name", models.CharField(max_length=20)),
(
"rider",
models.ForeignKey(
f"{app_label}.Rider", on_delete=models.CASCADE
),
),
(
"name_and_id",
models.GeneratedField(
expression=Concat(("name"), ("rider_id")),
output_field=models.TextField(),
db_persist=True,
),
),
],
),
],
)
Pony = project_state.apps.get_model(app_label, "Pony")
Rider = project_state.apps.get_model(app_label, "Rider")
rider = Rider.objects.create()
pony = Pony.objects.create(name="pony", rider=rider)
self.assertEqual(pony.name_and_id, str(pony.name) + str(rider.id))

new_rider = Rider.objects.create()
pony.rider = new_rider
pony.save()
pony.refresh_from_db()
self.assertEqual(pony.name_and_id, str(pony.name) + str(new_rider.id))

def test_add_charfield(self):
"""
Tests the AddField operation on TextField.
Expand Down

0 comments on commit 41f26da

Please sign in to comment.