Opened 7 months ago

Closed 6 months ago

Last modified 6 months ago

#35359 closed Bug (fixed)

Got `django.core.exceptions.FieldError` when adding GeneratedField

Reported by: wd0517 Owned by: Bhuvnesh
Component: Migrations Version: 5.0
Severity: Release blocker Keywords: GeneratedField
Cc: David Sanders, Simon Charette 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

Here is an exists model

class Foo1(models.Model):
    age = models.IntegerField()

Then adding GeneratedField

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

Run makemigrations get below file

class Migration(migrations.Migration):

    dependencies = [
        ('temp_django_test', '0006_foo1'),
    ]

    operations = [
        migrations.AddField(
            model_name='foo1',
            name='lower_name',
            field=models.GeneratedField(db_persist=True, expression=django.db.models.functions.text.Lower('name'), output_field=models.CharField(max_length=11)),
        ),
        migrations.AddField(
            model_name='foo1',
            name='name',
            field=models.CharField(default='a', max_length=10),
            preserve_default=False,
        ),
    ]

Then run migrate, will got error

temp-django-5.0 ❯ python manage.py migrate
Operations to perform:
  Apply all migrations: admin, auth, contenttypes, sessions, temp_django_test
Running migrations:
  Applying temp_django_test.0007_foo1_lower_name_foo1_name...Traceback (most recent call last):
  File "/Users/wangdi/Desktop/test_scripts.nosync/temp-django-5.0/manage.py", line 22, in <module>
    main()
  File "/Users/wangdi/Desktop/test_scripts.nosync/temp-django-5.0/manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/Users/wangdi/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/Users/wangdi/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/Users/wangdi/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/core/management/base.py", line 413, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/core/management/base.py", line 459, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/core/management/base.py", line 107, in wrapper
    res = handle_func(*args, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/core/management/commands/migrate.py", line 356, in handle
    post_migrate_state = executor.migrate(
                         ^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/migrations/executor.py", line 135, in migrate
    state = self._migrate_all_forwards(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards
    state = self.apply_migration(
            ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/migrations/executor.py", line 252, in apply_migration
    state = migration.apply(state, schema_editor)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/migrations/migration.py", line 132, in apply
    operation.database_forwards(
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/migrations/operations/fields.py", line 108, in database_forwards
    schema_editor.add_field(
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/backends/mysql/schema.py", line 103, in add_field
    super().add_field(model, field)
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 709, in add_field
    definition, params = self.column_sql(model, field, include_default=True)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 369, in column_sql
    " ".join(
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 336, in _iter_column_sql
    generated_sql, generated_params = self._column_generated_sql(field)
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 434, in _column_generated_sql
    expression_sql, params = field.generated_sql(self.connection)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/models/fields/generated.py", line 58, in generated_sql
    resolved_expression = self.expression.resolve_expression(
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/models/expressions.py", line 975, in resolve_expression
    c.source_expressions[pos] = arg.resolve_expression(
                                ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/models/expressions.py", line 854, in resolve_expression
    return query.resolve_ref(self.name, allow_joins, reuse, summarize)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/models/sql/query.py", line 2014, in resolve_ref
    join_info = self.setup_joins(
                ^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/models/sql/query.py", line 1867, in setup_joins
    path, final_field, targets, rest = self.names_to_path(
                                       ^^^^^^^^^^^^^^^^^^^
  File "/Users/wd0517/Desktop/test_scripts.nosync/temp-django-5.0/.venv/lib/python3.11/site-packages/django/db/models/sql/query.py", line 1772, in names_to_path
    raise FieldError(
django.core.exceptions.FieldError: Cannot resolve keyword 'name' into field. Choices are: age, id, lower_name

Environment:

  • Python 3.11.8
  • Django 5.0.4
  • MySQL 8.0.34

I think the bug is caused by the order of operations in Migration, after adjusting it, then can be migrate successfully

Change History (13)

comment:1 by Simon Charette, 7 months ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

The auto-detector has to be adapted to make the AddField of generated field depend on any other AddField or AlterField of field it references through .expression. Since GeneratedField cannot depend on other GeneratedField the easier way of doing to avoid introspecting .expression is likely to make them depend on all other non-GeneratedField fields being added per model so they are always last.

comment:2 by David Sanders, 7 months ago

For anyone picking up this ticket: Simon & I were discussing ways to get fields from expressions in this PR: https://github.com/django/django/pull/16054

comment:3 by David Sanders, 7 months ago

Cc: David Sanders added

comment:4 by Simon Charette, 7 months ago

Cc: Simon Charette added

Thanks David, I knew this was implemented somewhere but I lost track of where exactly!

I think that relying on this property once it lands is definitely the way to go so it's reused by multiple code paths but in the case of stable/5.x we might be better off by always adding them last. Thoughts?

comment:5 by David Sanders, 7 months ago

Move "assigned tenant users" into "assigned client users" - need to figure out because acu requires an agreement

Hm can't fks depend on these though because the constraints for those are also grouped together last. 🤔

Version 0, edited 7 months ago by David Sanders (next)

comment:6 by Simon Charette, 7 months ago

Hm can't fks depend on these though because the constraints for those are also grouped together last. 🤔

Right. Since GeneratedField cannot be foreign keys though it should still allow for the following grouping of fields?

non_fk_non_generated_field | fk_fields | generated_fields

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

comment:7 by Bhuvnesh, 7 months ago

Has patch: set
Owner: changed from nobody to Bhuvnesh
Status: newassigned

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

In 97d48cd:

Refs #34007, Refs #35359 -- Added Q.referenced_based_fields property.

Thank you to Mariusz Felisiak and Natalia Bidart for the reviews.

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

Resolution: fixed
Status: assignedclosed

In 9aeb38c2:

Fixed #35359 -- Fixed migration operations ordering when adding fields referenced by GeneratedField.expression.

Thank you to Simon Charette for the review.

comment:10 by Natalia Bidart, 6 months ago

Triage Stage: AcceptedReady for checkin

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

In fa202d5c:

[5.0.x] Refs #34007, Refs #35359 -- Added Q.referenced_based_fields property.

Thank you to Mariusz Felisiak and Natalia Bidart for the reviews.

Backport of 97d48cd3c6f409584b5cc19fbddfca917bae78fd from main

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

In 24f54c3:

[5.0.x] Fixed #35359 -- Fixed migration operations ordering when adding fields referenced by GeneratedField.expression.

Thank you to Simon Charette for the review.

Backport of 9aeb38c296c69532c7e64b5e3d706a5eb17b3f12 from main.

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

In ac9e18f:

[5.0.x] Refs #35359 -- Fixed OperationTests.test_add_generate_field() test on PostgreSQL.

Concat() in Django 5.0 is not immutable on PostgreSQL and cannot be used
in GeneratedField, see 6364b6ee1071381eb3a23ba6b821fc0d6f0fce75.

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