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.
  • Loading branch information
DevilsAutumn authored and sarahboyce committed May 2, 2024
1 parent 39828fa commit 9af89b0
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 1 deletion.
35 changes: 35 additions & 0 deletions django/db/migrations/autodetector.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ def _detect_changes(self, convert_apps=None, graph=None):
self.altered_indexes = {}
self.altered_constraints = {}
self.renamed_fields = {}
self.already_added_fields = set()

# Prepare some old/new state and model lists, separating
# proxy models and ignoring unmigrated apps.
Expand Down Expand Up @@ -1109,6 +1110,9 @@ def generate_added_fields(self):
self._generate_added_field(app_label, model_name, field_name)

def _generate_added_field(self, app_label, model_name, field_name):
if (app_label, model_name, field_name) in self.already_added_fields:
return

field = self.to_state.models[app_label, model_name].get_field(field_name)
# Adding a field always depends at least on its removal.
dependencies = [
Expand Down Expand Up @@ -1153,6 +1157,36 @@ def _generate_added_field(self, app_label, model_name, field_name):
and callable(field.default)
):
self.questioner.ask_unique_callable_default_addition(field_name, model_name)

if field.generated:
referenced_base_fields = {
child.split(models.constants.LOOKUP_SEP, 1)[0]
for child in models.sql.query.get_children_from_q(
models.Q(field.expression)
)
}
newly_added_fields = sorted(self.new_field_keys - self.old_field_keys)
for ref_field_name in referenced_base_fields:
if (app_label, model_name, ref_field_name) not in newly_added_fields:
continue
self._generate_added_field(app_label, model_name, ref_field_name)

# Add dependencies on the fks so that the Generated field is added even
# after the fks.
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:
dependencies.append(
OperationDependency(
app_label,
model_name,
added_field.name,
OperationDependency.Type.CREATE,
)
)

self.add_operation(
app_label,
operations.AddField(
Expand All @@ -1163,6 +1197,7 @@ def _generate_added_field(self, app_label, model_name, field_name):
),
dependencies=dependencies,
)
self.already_added_fields.add((app_label, model_name, field_name))

def generate_removed_fields(self):
"""Make RemoveField operations."""
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 9af89b0

Please sign in to comment.