System check when both unique_together and UniqueConstraint are used

Django Internals ORM
1

Hi all,

With unique_together likely going away in the future in favour of UniqueConstraint, I noticed that Django does not raise a system check error or warning when you use both for the same set of fields on the same model. For example:

class TranslatableText(models.Model):
    translation_key = models.UUIDField(default=uuid.uuid4, editable=False)
    locale = models.CharField(max_length=8)
    content = models.TextField()

    class Meta:
        unique_together = [("translation_key", "locale")]
        constraints = [
            models.UniqueConstraint(
                fields=["translation_key", "locale"],
                name="unique_translation_key_locale",
            ),
        ]

It might be obvious that you shouldn’t do this. However, it’s not immediately obvious in the case of abstract models (e.g. from a third-party package that provides i18n) if you inherit the abstract model’s Meta as well. Without knowing that there’s a unique_together or a UniqueConstraint already in place, you might end up adding the other one and end up with two constraints for the same set of fields.

As a package maintainer, it’s pretty easy to add the system check yourself, but I’m wondering whether this should be included in Django itself. If so, I’d be happy to create a ticket and patch.

Although one might also argue it’s not worth to add the check into Django, as we’ll likely add another check to raise a deprecation warning about unique_together when it actually is going away anyway.

Another point is that Django also doesn’t raise an error if you have more than one UniqueConstraints with the same set of fields. This is understandable though, as UniqueConstraint has other options e.g. condition which means the constraints may have different purposes despite using the same fields. With unique_together, Django seems smart enough to consolidate duplicate entries into one when creating migrations (because they get converted to a set).

What do you think, shall we add such check(s)? Keen to hear your thoughts.

Thanks!

2

I think theres a good argument here for a general check for duplicate constraints or indexes. Even if Django doesn’t end up creating duplicates that could still be an issue for things like determining the correct name or migrations that remove one index/constraint.

Then even when we deprecate unique_together the check could still have value.

2 Likes