Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#30350 closed Bug (fixed)

Migration re-add check constraint continuously when check condition contains a range object.

Reported by: Sigurd Ljødal Owned by: Florian Apolloner
Component: Migrations Version: 2.2
Severity: Release blocker Keywords:
Cc: Ian Foote Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A CheckConstraint with a Q(x__in=range(y, z)) condition is repeatedly deleted and re-added when running makemigrations.

models.CheckConstraint(
    check=models.Q(month__in=range(1, 13)),
    name='check_valid_month',
)

The generated migration looks like this, so I suspect that the issue is because the range is converted into a tuple:

    operations = [
        migrations.RemoveConstraint(
            model_name='monthlybudget',
            name='check_valid_month',
        ),
        migrations.AddConstraint(
            model_name='monthlybudget',
            constraint=models.CheckConstraint(check=models.Q(month__in=(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)), name='check_valid_month'),
        ),
    ]

A sample project with this issue can be found here:
https://github.com/ljodal/djangocon-eu-2019

I'm open to fixing this issue myself, but I would like to clarify what would be a correct fix to this issue. I see at least two possible solutions, maybe three:

  1. Keep the range method call in the generated migration file
  2. Disallow using ranges in check constraints
  3. (At least on PostgreSQL, we could use a range expression in the database too.)

Change History (8)

comment:1 Changed 4 months ago by felixxm

In check constraints you can use range lookup and it works fine, e.g.

models.CheckConstraint(
    check=models.Q(month__range=[1, 13]),
    name='check_valid_month',
)

also casting an iterator to a list works good, e.g.

models.CheckConstraint(
    check=models.Q(month__in=list(range(1, 13))),
    name='check_valid_month',
)

so in this case iterator is an issue. I would check later if this can be easily fix and if not we should document this limitation.

comment:2 Changed 4 months ago by felixxm

Cc: Ian Foote added
Component: Database layer (models, ORM)Migrations
Severity: NormalRelease blocker
Summary: CheckConstraint repeatedly deleted and re-addedMigration re-add check constraint continuously when check condition contains iterator.
Triage Stage: UnreviewedAccepted

comment:3 Changed 4 months ago by felixxm

Owner: changed from nobody to felixxm
Status: newassigned

comment:4 Changed 4 months ago by Sigurd Ljødal

Just to confirm, I modified the migration to use range() and makemigrations no longer detects any changes.

comment:5 Changed 4 months ago by felixxm

Owner: changed from felixxm to Florian Apolloner
Triage Stage: AcceptedReady for checkin

comment:6 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 2e38f20:

Fixed #30350 -- Prevented recreation of migration for operations with a range object.

Thanks to Mariusz Felisiak for helping with the patch.

comment:7 Changed 4 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 896cc719:

[2.2.x] Fixed #30350 -- Prevented recreation of migration for operations with a range object.

Thanks to Mariusz Felisiak for helping with the patch.

Backport of 2e38f2015aba224b68a91a3012b87223f3046bb6 from master.

comment:8 Changed 4 months ago by Simon Charette

Summary: Migration re-add check constraint continuously when check condition contains iterator.Migration re-add check constraint continuously when check condition contains a range object.
Note: See TracTickets for help on using tickets.
Back to Top