Custom field's db_check() not taken into account when deciding whether to recreate constraint.

[Baptiste Mispelon]

This is a regression introduced in 9953c804a9375956a542da94665662d306dff48d.

I have a custom field that implements db_check() in order to add a custom db constraint:

class CharChoiceField(models.CharField):
    """
    A custom CharField that automatically creates a db constraint to guarante
    that the stored value respects the field's `choices`.
    """
    @property
    def non_db_attrs(self):
        # Remove `choices` from non_db_attrs so that migrations that only change
        # choices still trigger a db operation and drop/create the constraint.
        attrs = super().non_db_attrs
        return tuple({*attrs} - {"choices"})

    def db_check(self, connection):
        constraint = CheckConstraint(
            condition=Q(**{f"{self.name}__in": dict(self.choices)}),
            name="",  # doesn't matter, Django will reassign one anyway
        )
        with connection.schema_editor() as schema_editor:
            return constraint.get_check_sql(self.model, schema_editor)

This used to work well (in Django < 5.0): the constraint was added alongside the field when the initial migration was run, and any changes to the field's choices would trigger a new migration (that's standard Django behavior) which when executed would drop/create the constraint with the new value of choices.

However the commit I linked to at the beginning broke this: now any changes to choices still creates a migration, but executing the migration is now a no-op (which can be confirmed by checking with sqlmigrate).

I'm not sure if Field.db_check() can be considered public API (it's not documented), but I have a fix which is fairly simple (PR incoming) so I hope you'll consider it.

[Baptiste Mispelon]

Draft PR (missing tests and release notes) is here: ​https://github.com/django/django/pull/19412

[Natalia Bidart]

Thanks Baptiste for the details and PR! Could you please provide a failing test on main that demonstrates the regression? While this won’t be a release blocker since 5.0 is EOL, the test would still be valuable.

[Mariusz Felisiak]

Replying to Baptiste Mispelon:

I'm not sure if Field.db_check() can be considered public API (it's not documented), but I have a fix which is fairly simple (PR incoming) so I hope you'll consider it.

Unfortunately, the default implementation of db_check() already incorporates the column name in generated SQL, so using it will reintroduced issue fixed by 9953c804a9375956a542da94665662d306dff48d.

[Baptiste Mispelon]

Ah right, I didn't realize that my fix was just doing the %-formatting twice and therefore just turning off the optimization that commit 9953c804a9375956a542da94665662d306dff48d was adding (which explains why CI was failing on MariaDB). Maybe the fix is not as simple as I first thought (it was too good to be True!).

I'm attaching a patch that adds a failing test (targeting main). I tried to keep it simple but unfortunately it's still fairly verbose (it's quite hard to test this type of code).

[Sarah Boyce]

It looks like there isn't a simple fix here and this is not officially supported

I have found a forum discussion around wanting to be able to define a constraint for a custom field: ​https://forum.djangoproject.com/t/dynamically-populate-constraints-in-field-contribute-to-class/22934/5
A number of folks seem to be in favor of the change (will add them in cc), so I think we could have a "New Feature" ticket to add support for this

[Carlton Gibson]

I agree this looks like a new feature. I'll accept on that basis. (Happy if someone wants to reclassify.)