#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 , 4 months ago
Description: | modified (diff) |
---|
comment:2 by , 4 months ago
Keywords: | unique constraint validation expressions added; uniqueconstraint removed |
---|---|
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 4 months ago
Cc: | added |
---|
comment:4 by , 4 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
follow-up: 8 comment:5 by , 4 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 , 4 months ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:7 by , 4 months ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
comment:8 by , 4 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 , 4 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 , 4 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:11 by , 4 months ago
Thanks for the guidance. I appreciate the advice and will follow it moving forward.
comment:12 by , 4 months ago
Patch needs improvement: | unset |
---|---|
Type: | New feature → Bug |
comment:13 by , 4 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=False
all expressions checks should be turned intoOR (rhs IS NULL AND lhs IS NULL)
due to the lack of general support for theIS DISTINCT FROM
operator.Is turned into