squashmigrations optimizer crashes when fields' names are swapped using a temporary name

[InvalidInterrupt]

If you rename fields using a pattern like a->c; b->a; c->b (such as if previously DateTimeFields using auto_now and auto_now_add had been mixed-up) and then attempt to squashmigrations with an optimization barrier between the CreateModel and RenameFields, the migration optimizer will attempt to create a CreateModel operation object with two fields using the same name and fail. I'll attach a migration file that triggers the failure.

I believe the root cause of this issue is that django.db.migrations.operations.fields.RenameField allows itself to optimize through (i.e be moved to the right of, I may have gotten this terminology wrong) other RenameField operations that reference old_name.

[InvalidInterrupt]

example migration

[InvalidInterrupt]

I don't have time to write a patch and tests myself at the moment. I'll assign myself once that changes, unless someone else picks this up first.

[Mariusz Felisiak]

Thanks for the report.

It's squashed to a single CreateModel() operation when the initial migration doesn't contain migrations.RunPython(), but crashes if it does:

Traceback (most recent call last):
  File "manage.py", line 22, in <module>
    main()
  File "manage.py", line 18, in main
    execute_from_command_line(sys.argv)
  File "/django/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/django/django/core/management/__init__.py", line 413, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/django/django/core/management/base.py", line 354, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/django/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "/django/django/core/management/commands/squashmigrations.py", line 145, in handle
    new_operations = optimizer.optimize(operations, migration.app_label)
  File "/django/django/db/migrations/optimizer.py", line 34, in optimize
    result = self.optimize_inner(operations, app_label)
  File "/django/django/db/migrations/optimizer.py", line 47, in optimize_inner
    result = operation.reduce(other, app_label)
  File "/django/django/db/migrations/operations/models.py", line 233, in reduce
    CreateModel(
  File "/django/django/db/migrations/operations/models.py", line 55, in __init__
    _check_for_duplicates('fields', (name for name, _ in self.fields))
  File "/django/django/db/migrations/operations/models.py", line 17, in _check_for_duplicates
    raise ValueError(
ValueError: Found duplicate value field_a in CreateModel fields argument.

[Mariusz Felisiak]

Sample project.

[Mariusz Felisiak]

I attached a sample project, manage.py squashmigrations test_one 0001 0004 reproduces this issue.

[Hasan Ramezani]

​PR

[Hasan Ramezani]

Comment from @InvalidInterrupt on PR:

Thanks to having your test as reference I put together my idea for fixing this in RenameField.reduce() here:
https://github.com/InvalidInterrupt/django/tree/ticket_32256.

I think it's better to fix this there rather than try to prevent the optimizer from considering such optimizations in the first case; other proposed features I've seen in trac (i.e. https://code.djangoproject.com/ticket/27844) would be likely to cause the issue to arise again. My approach does imply that this sequence of migrations is not optimizable; I would be happy to be proven wrong if someone identified a way to optimize this in the general case.

I'm (probably overly) concerned about stepping on toes here, so I'm not making another PR. I may find myself busy again over the next couple days; please feel free to either ask me to make a PR or just take my commit if that's easier for you all.

I am not sure about the solution proposed by @InvalidInterrupt.
@Mariusz Could you please clarify it? If you agree with the proposed solution I can update the PR with the proposed solution.

Thanks

[Mariusz Felisiak]

I am not sure about the solution proposed by @InvalidInterrupt.

Sounds reasonable.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 7c18b22e:

Fixed #32256 -- Fixed migration optimization crash when swapping field names.

This disables optimization of RenameField operation when an old field
name is referenced in subsequent operations.

Co-authored-by: InvalidInterrupt <InvalidInterrupt@users.noreply.github.com>