Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #35359 -- Made autodetector add fields referenced by GeneratedField.expression before it. #18068

Merged
merged 1 commit into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions django/db/migrations/autodetector.py
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
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
``GeneratedField`` was added before any of the referenced fields from its
``expression`` definition (:ticket:`35359`).
77 changes: 77 additions & 0 deletions tests/migrations/test_autodetector.py
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"))
)

sarahboyce marked this conversation as resolved.
Show resolved Hide resolved
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
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_add_generate_field"
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