Opened 3 months ago

Closed 6 weeks ago

Last modified 6 weeks ago

#36438 closed Cleanup/optimization (fixed)

makemigrations removes fields in wrong order when GeneratedFields are involved

Reported by: Colton Saska Owned by: Clifford Gama
Component: Migrations Version: 5.2
Severity: Normal Keywords: generatedfield removefield
Cc: Colton Saska 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

Consider a model like PayItem with fields quantity, rate and total.

When total is a GeneratedField dependent on quantity, if you remove quantity and total and generate migrations you get a migration like

    operations = [
        migrations.RemoveField(
            model_name="payitem",
            name="total",
        ),
        migrations.RemoveField(
            model_name="payitem",
            name="quantity",
        ),
    ]

which will throw django.db.utils.OperationalError: (3108, "Column 'amount' has a generated column dependency.").

It'd be nice if makemigrations listed operations in the correct order.

Change History (8)

in reply to:  description comment:1 by Colton Saska, 3 months ago

Replying to csaska:

Consider a model like PayItem with fields quantity, rate and total.

When total is a GeneratedField dependent on quantity, if you remove quantity and total and generate migrations you get a migration like

    operations = [
        migrations.RemoveField(
            model_name="payitem",
            name="total",
        ),
        migrations.RemoveField(
            model_name="payitem",
            name="quantity",
        ),
    ]

which will throw django.db.utils.OperationalError: (3108, "Column 'amount' has a generated column dependency.").

It'd be nice if makemigrations listed operations in the correct order.

In my example, I actually had fields amount, rate and total. Perhaps it using alphabetical order and thus dropping amount operation comes before dropping total operation?

comment:2 by Simon Charette, 3 months ago

Component: Database layer (models, ORM)Migrations
Easy pickings: unset
Keywords: generatedfield removefield added
Triage Stage: UnreviewedAccepted

I haven't reproduced myself but given I can't find any logic in AutoDetector._generate_removed_field to ensure that GeneratedField removals depend on removals of fields they reference this seems legitimate. I suspect field addition through makemigrations suffers from the same problem and that the optimizer can also result in ordering issues because FieldOperation.references_field doesn't consider generated field references.

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

comment:3 by Clifford Gama, 3 months ago

Owner: set to Clifford Gama
Status: newassigned

comment:4 by Clifford Gama, 2 months ago

Has patch: set

comment:5 by Sarah Boyce, 6 weeks ago

Needs tests: set

comment:6 by Sarah Boyce, 6 weeks ago

Needs tests: unset
Triage Stage: AcceptedReady for checkin

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 6 weeks ago

Resolution: fixed
Status: assignedclosed

In 1a7fc0f:

Fixed #36438 -- Made MigrationAutodetector remove generated fields before their base fields.

Thanks to Colton Saska for the report and to Simon Charette for the review.

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

In 45ba7683:

Refs #36438 -- Made FieldOperation.references_field() detect references in GeneratedField.expression.

Thanks to Simon Charette for the suggestion and review.

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