Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#33192 closed Bug (worksforme)

It is not possible to use a custom lookup/transorm in a CheckConstraint

Reported by: Fabien MICHEL Owned by: nobody
Component: Database layer (models, ORM) Version: 3.1
Severity: Normal Keywords: lookup, transform, CheckContraint, migrate
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Fabien MICHEL)

Using a custom lookup or transform in a CheckContraint make migrate command fail.

class MonthLastDay(Transform):
    lookup_name = "month_last_day"
    output_field = DateField()
    template = "(DATE_TRUNC('month', %(expressions)s) + (interval '1 month - 1 day'))::date"

DateField.register_lookup(MonthLastDay)

class MyModel(models.Model):
    class Meta:
        constraints = [
            CheckConstraint(check=Q(date__month_last_day__gte=F("period_start")), name="date_month_end_after_period_start"),
        ]

   date = models.DateField()
   period_start = models.DateField()

django makemigrations command does not indicate any issue with this and make use of month_last_day lookup as if it know about it.

Migration command for this contraint:

migrations.AddConstraint(
    model_name='mymodel',
    constraint=models.CheckConstraint(
        check=models.Q(
            ('date__month_last_day__gte', django.db.models.expressions.F('period_start'))
        ), 
        name='date_month_end_after_period_start'),
),

django migrate command fail indicating that date__month_last_day join is not possible

django.core.exceptions.FieldError: Joined field references are not permitted in this query

I'm not sure what could be done. May be the documentation should warn to not use custom lookup/transform in CheckContraint.

Change History (9)

comment:1 by Fabien MICHEL, 3 years ago

Description: modified (diff)

comment:2 by Fabien MICHEL, 3 years ago

Description: modified (diff)

comment:3 by Fabien MICHEL, 3 years ago

Description: modified (diff)

comment:4 by Mariusz Felisiak, 3 years ago

Resolution: worksforme
Status: newclosed

It works for me when you register a custom lookup as documented:

"You will need to ensure that this registration happens before you try to create any querysets using it. You could place the implementation in a models.py file, or register the lookup in the ready() method of an AppConfig"

comment:5 by Fabien MICHEL, 3 years ago

Thanks for your answer.

If I understand, I should copy the custom lookup into the migration file to have it available during migration process ?

in reply to:  5 comment:6 by Mariusz Felisiak, 3 years ago

Replying to Fabien MICHEL:

If I understand, I should copy the custom lookup into the migration file to have it available during migration process ?

No, you "....need to ensure that this registration happens before you try to create any querysets using it.", so you can call register_lookup() in your models.py or AppConfig.ready().

comment:7 by Fabien MICHEL, 3 years ago

In this case, won't I expose my migration to use code that could be ananchronic if the code of the custom lookup changes after the migration has been created ?
Is it not better to embed the custom lookup on the migration which create the constraint in database ?

Last edited 3 years ago by Fabien MICHEL (previous) (diff)

comment:8 by Simon Charette, 3 years ago

In this case, won't I expose my migration to use code that could be ananchronic if the code of the custom lookup changes after the migration has been created? Is it not better to embed the custom lookup on the migration which create the constraint in database ?

The migration framework doesn't detect change at the resulting SQL level so if you were to change your lookup logic you'd have to manually create a migration that removes and adds back the new constraint. Nothing prevents you from defining the lookup in two locations though (ready()) and in the migration itself so it's already there and only overrides the project specific lookup when the migration file is loaded. Just keep in mind that Django won't be hand holding you if you change the SQL generated by the lookup.

In a sense this is similar to how Django allows you to add and remove managers to your model definition without 'baking' the manage definition in the migration file; since the migration framework holds 'references' of managers and lookups it's your responsibility to keep 'referenced values' coherent with migration states.

comment:9 by Fabien MICHEL, 3 years ago

It is all clear for me, thanks you. Shouldn't we add a warning about this in CheckConstraint documentation ?
Here is a proposal:

.. note::

    If you use a :doc:`custom lookup </howto/custom-lookups>` in CheckContraint
    expression you will have to ensure the expected custom lookup is registered
    before the migration run. Therefore, the migration framework doesn't detect
    change at the resulting SQL of the expression. It is up to you to keep 
    referenced lookup coherent with migration state.
Note: See TracTickets for help on using tickets.
Back to Top