Argument based equality check of BaseExpression causes unstable migration state

[bacilla]

Changing a model's meta option ordering that contains expressions causes Django to generate an infinite number of migrations.

Test model:

from django.db import models
from django.db.models.functions import Cast, Concat, Right


class TestEntity(models.Model):
    some_property = models.TextField()

    class Meta:
        ordering = ('some_property',)

Makemigrations log:

$ python manage.py makemigrations app
Migrations for 'app':
  app/migrations/0001_initial.py
    - Create model TestEntity

Then ordering have been changed.

class TestEntity(models.Model):
    some_property = models.TextField()

    class Meta:
        ordering = (
            Cast(models.Func(models.F('some_property'), models.Value('^\d+'), function='SUBSTRING'), models.IntegerField()),
            Right(Concat(models.Value('00000'), models.F('some_property')), models.Value(5))
        )

Makemigrations log:

$ python manage.py makemigrations app
Migrations for 'app':
  app/migrations/0002_auto_20180907_1712.py
    - Change Meta options on testentity
$ python manage.py makemigrations app
Migrations for 'app':
  app/migrations/0003_auto_20180907_1713.py
    - Change Meta options on testentity
$ python manage.py makemigrations app
Migrations for 'app':
  app/migrations/0004_auto_20180907_1713.py
    - Change Meta options on testentity

[bacilla]

test project

[Simon Charette]

Thanks for filling up a new bug report.

This is caused by the fact models.Field.__eq__ ​is based of creation_counter which makes models.IntegerField() != models.IntegerField() and thus Cast(expr, models.IntegerField()) != Cast(expr, models.IntegerField()).

I feel like this is going to be hard to solve at the Field level without revisiting how we determine field definition ordering. This has always been a hack to work around unordered type.__new__(attrs) anyway which is not necessary in Python 3.6+ and can be worked around in Python 3.5+ thanks to ​PEP 520's type.__prepare__ which could be added to ModelBase.

Another solution could be to special case output_field equality checks in BaseExpression.__eq__ ​even more. This would surely be a less invasive change if we plan to backport the adjustment.

Related to #26257.

[bacilla]

Seems like a working hack.

class ToInteger(models.IntegerField):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.creation_counter = 1


class TestEntity(models.Model):
    some_property = models.TextField()

    class Meta:
        ordering = (
            Cast(models.Func(models.F('some_property'), models.Value('^\d+'), function='SUBSTRING'), ToInteger()),
            Right(Concat(models.Value('00000'), models.F('some_property')), models.Value(5)),
        )

[Simon Charette]

Patch which is still missing a few more tests

​https://github.com/django/django/pull/10378

[Simon Charette]

@bacilla by the way a less invasive workaround is to pass Cast(output_field) as a keyword argument instead of a positional one.

class TestEntity(models.Model):
    some_property = models.TextField()

    class Meta:
        ordering = (
            Cast(models.Func(models.F('some_property'), models.Value('^\d+'), function='SUBSTRING'), output_field=models.IntegerField()),
            Right(Concat(models.Value('00000'), models.F('some_property')), models.Value(5))
        )

[Tim Graham <timograham@…>]

In bc7e288:

Fixed #29745 -- Based Expression equality on detailed initialization signature.

The old implementation considered objects initialized with an equivalent
signature different if some arguments were provided positionally instead of
as keyword arguments.

Refs #11964, #26167.