#35594 closed Bug (fixed)
Add support for non-distinct NULL expressions to UniqueConstraint.validate()
| Reported by: | Mark Gensler | Owned by: | Simon Charette |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 5.0 |
| Severity: | Release blocker | Keywords: | unique constraint nulls_distinct validation expressions |
| Cc: | Simon Charette | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Expressions which evaluate to NULL within UniqueConstraint(*expressions, nulls_distinct=False) are still treated as distinct by UniqueConstraint.validate(). This means a ValidationError is not raised when it should be.
Similarly, if the database connection uses interprets_empty_strings_as_nulls this is also ignored by .validate().
Should #35575 be merged, this problem would also extend to any GeneratedField included in UniqueConstraint(fields=[...], nulls_distinct=False).
E.g.
class Book(models.Model):
name = CharField(max_length=255, null=True, blank=True)
class Meta:
constraints = [
UniqueConstraint(F("name"), nulls_distinct=False, name="book_name_null_unique")
]
then
> Book.objects.create(name=None) > book = Book(name=None) > book.full_clean() # Should raise a `ValidationError` but doesn't. > book.save() # The database raises a `UniqueViolation`.
This ticket was raised following discussion in https://github.com/django/django/pull/18356#pullrequestreview-2166340541
Change History (16)
comment:1 by , 16 months ago
| Description: | modified (diff) |
|---|
comment:2 by , 16 months ago
| Keywords: | unique constraint validation expressions added; uniqueconstraint removed |
|---|---|
| Severity: | Normal → Release blocker |
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 16 months ago
| Cc: | added |
|---|
comment:4 by , 16 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
follow-up: 8 comment:5 by , 16 months ago
Dong it would have been appreciate if you could have chimed in with Mark and myself before jumping on assignment.
Worked on a PoC here.
comment:6 by , 16 months ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
comment:7 by , 16 months ago
| Has patch: | set |
|---|---|
| Patch needs improvement: | set |
comment:8 by , 16 months ago
Replying to Simon Charette:
Sorry for jumping in without prior discussion. I wasn't aware and will be more cautious in the future.
follow-up: 11 comment:9 by , 16 months ago
No worries and no harm done, I just wanted to make sure you wouldn't waste time on this problem since both Mark and I had a pretty good idea of where to take the patch.
I guess a possible rule of thumb would be to at least wait a few days after the creation of a ticket if there seems to be active discussions and no call for someone to take the implementation work. In doubt, asking if help is required is also always welcome.
comment:10 by , 16 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:11 by , 16 months ago
Thanks for the guidance. I appreciate the advice and will follow it moving forward.
comment:12 by , 16 months ago
| Patch needs improvement: | unset |
|---|---|
| Type: | New feature → Bug |
comment:13 by , 16 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Thanks for creating the report!
This was missed in #34701. Elevating to release blocker as this is a bug in a newly released feature.
I guess solution would be that when
nulls_distinct=Falseall expressions checks should be turned intoOR (rhs IS NULL AND lhs IS NULL)due to the lack of general support for theIS DISTINCT FROMoperator.Is turned into