Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#35422 closed Bug (fixed)

Migration crash when renaming a field referenced in GeneratedField.expression

Reported by: Sarah Boyce Owned by: Mariusz Felisiak
Component: Migrations Version: 5.0
Severity: Release blocker Keywords:
Cc: Simon Charette, Mariusz Felisiak, Lily Foote, David Sanders, Bhuvnesh Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Given a model (migrated) on SQLite

class Foo(models.Model):
    name = models.CharField(max_length=10)
    lower_name = models.GeneratedField(
        expression=Lower("name"),
        output_field=models.CharField(max_length=10),
        db_persist=True,
    )

when you update to

class Foo(models.Model):
    surname = models.CharField(max_length=10)
    lower_name = models.GeneratedField(
        expression=Lower("surname"),
        output_field=models.CharField(max_length=10),
        db_persist=True,
    )

and makemigrations, you then get asked Was foo.name renamed to foo.surname (a CharField)? [y/N] and say y (because it was)
You then get a crash when migrating:

Operations to perform:
  Apply all migrations: app3
Running migrations:
  Applying app3.0005_rename_name_foo_surname_alter_foo_lower_name...Traceback (most recent call last):
  File "path_to_project\mysite\manage.py", line 22, in <module>
    main()
  File "path_to_project\mysite\manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "path_to_project\venv\Lib\site-packages\django\core\management\__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "path_to_project\venv\Lib\site-packages\django\core\management\__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "path_to_project\venv\Lib\site-packages\django\core\management\base.py", line 413, in run_from_argv
    self.execute(*args, **cmd_options)
  File "path_to_project\venv\Lib\site-packages\django\core\management\base.py", line 459, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\core\management\base.py", line 107, in wrapper
    res = handle_func(*args, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\core\management\commands\migrate.py", line 356, in handle
    post_migrate_state = executor.migrate(
                         ^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\migrations\executor.py", line 135, in migrate
    state = self._migrate_all_forwards(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\migrations\executor.py", line 167, in _migrate_all_forwards
    state = self.apply_migration(
            ^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\migrations\executor.py", line 255, in apply_migration
    state = migration.apply(state, schema_editor)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\migrations\migration.py", line 132, in apply
    operation.database_forwards(
  File "path_to_project\venv\Lib\site-packages\django\db\migrations\operations\fields.py", line 241, in database_forwards
    schema_editor.alter_field(from_model, from_field, to_field)
  File "path_to_project\venv\Lib\site-packages\django\db\backends\base\schema.py", line 875, in alter_field
    or old_field.generated_sql(self.connection)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\models\fields\generated.py", line 58, in generated_sql
    resolved_expression = self.expression.resolve_expression(
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\models\expressions.py", line 1052, in resolve_expression
    c.source_expressions[pos] = arg.resolve_expression(
                                ^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\models\expressions.py", line 874, in resolve_expression
    return query.resolve_ref(self.name, allow_joins, reuse, summarize)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\models\sql\query.py", line 2010, in resolve_ref
    join_info = self.setup_joins(
                ^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\models\sql\query.py", line 1863, in setup_joins
    path, final_field, targets, rest = self.names_to_path(
                                       ^^^^^^^^^^^^^^^^^^^
  File "path_to_project\venv\Lib\site-packages\django\db\models\sql\query.py", line 1768, in names_to_path
    raise FieldError(
django.core.exceptions.FieldError: Cannot resolve keyword 'name' into field. Choices are: id, lower_name, surname

Change History (11)

comment:1 by Natalia Bidart, 6 months ago

Cc: Simon Charette Mariusz Felisiak Lily Foote added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thank you Sarah, I reproduced and confirmed that this is not a dupe of #35359 (or at least the fix for that ticket is not fixing this issue).

This is a release blocker for 5.0 given that GeneratedField was added in that Django version. I also reproduced in f333e3513e8bdf5ffeb6eeb63021c230082e6f95.

comment:2 by Natalia Bidart, 6 months ago

Cc: David Sanders Bhuvnesh added

comment:3 by Simon Charette, 6 months ago

I don't think the problem is specific to SQLite.

The non-invasive back portable solution I can think of is for FieldError to be caught and also result in a ValueError pointing out that the generated field must be removed first.

In other words, if we can't generate the SQL for the old field anymore but we can for the new one then it was certainly changed and the same lack of alteration support limitation applies.

Last edited 6 months ago by Simon Charette (previous) (diff)

comment:4 by Sarah Boyce, 6 months ago

Summary: Migration crash when renaming a field referenced in GeneratedField.expression on SQLiteMigration crash when renaming a field referenced in GeneratedField.expression

In other words, if we can't generate the SQL for the old field anymore but we can for the new one then it was certainly changed and the same lack of alteration support limitation applies.

Makes sense! Agree that I found this on SQLite is irrelevant

comment:5 by Mariusz Felisiak, 6 months ago

Owner: changed from nobody to Mariusz Felisiak
Status: newassigned

comment:6 by Mariusz Felisiak, 6 months ago

Has patch: set

comment:7 by Sarah Boyce, 6 months ago

Triage Stage: AcceptedReady for checkin

comment:8 by Sarah Boyce <42296566+sarahboyce@…>, 6 months ago

Resolution: fixed
Status: assignedclosed

In 91a4b9a8:

Fixed #35422 -- Fixed migrations crash when altering GeneratedField referencing rename field.

Thanks Sarah Boyce for the report and Simon Charette for the
implementation idea.

comment:9 by Sarah Boyce <42296566+sarahboyce@…>, 6 months ago

In c544f1a:

[5.0.x] Fixed #35422 -- Fixed migrations crash when altering GeneratedField referencing rename field.

Thanks Sarah Boyce for the report and Simon Charette for the
implementation idea.

Backport of 91a4b9a8ec2237434f06866f39c7977e889aeae6 from main.

comment:10 by nessita <124304+nessita@…>, 6 months ago

In e72049aa:

Refs #35422 -- Fixed typo in docs/releases/5.0.5.txt.

comment:11 by Natalia <124304+nessita@…>, 6 months ago

In e18e931:

[5.0.x] Refs #35422 -- Fixed typo in docs/releases/5.0.5.txt.

Backport of e72049aa6302411d8cdf2e798e15fa38e76b92fc from main.

Note: See TracTickets for help on using tickets.
Back to Top