[Django] #37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in condition

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 6.0 | Severity: Normal
Keywords: UniqueConstraint | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
== Summary
When a `UniqueConstraint` has a `condition` that references a nullable
field (e.g., `condition=Q(cash_register_type=10)` on a nullable
`PositiveSmallIntegerField`), Django's `UniqueConstraint.validate()`
incorrectly reports a constraint violation if the instance being saved has
`cash_register_type=None` and another record matching the condition
already exists.

''Tested in Django==5.2.4 (with Postgres 17) and 6.0.4 (with Postgres
18)''


== Steps to Reproduce

{{{
from django.db import models

class Location(models.Model):
name = models.CharField(max_length=100)

class Device(models.Model):
class CashRegisterType(models.IntegerChoices):
MASTER = 10, "Master"
SLAVE = 20, "Slave"
name = models.CharField(max_length=100)
pos_location = models.ForeignKey(Location, on_delete=models.CASCADE)
cash_register_type = models.PositiveSmallIntegerField(
choices=CashRegisterType,
blank=True, null=True, default=None,
)

class Meta:
constraints = [
models.UniqueConstraint(
fields=["pos_location"],
condition=models.Q(cash_register_type=10),
name="unique_master_cash_register",
violation_error_message="Only one Master per
POSLocation.",
)
]
}}}

Create one Device with `cash_register_type` Master and creating another
one for the same Location with `cash_register_type=None` will raise
ValidationError
{{{
from myapp.models import Location, Device

loc = Location.objects.create(name="Store 1")

# 1. Create a Master -- works
master = Device.objects.create(
name="CashRegister A", pos_location=loc, cash_register_type=10
)

# 2. Create a second Device with cash_register_type=None
# while a Master already exists -- BUG
register_b = Device.objects.create(
name="Device B", pos_location=loc, cash_register_type=None
)
}}}

== Workaround
Add an explicit `isnull=False` check to the condition:
`condition=models.Q(cash_register_type=10) &
models.Q(cash_register_type__isnull=False)`
Although `10` should already be `isnull=False`.
--
Ticket URL: <https://code.djangoproject.com/ticket/37057>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------

Reporter: Drews | Owner: (none)
Type: Bug | Status: closed
Component: Database layer | Version: 6.0
(models, ORM) | Resolution:
Severity: Normal | worksforme

Keywords: UniqueConstraint | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* resolution: => worksforme
* status: new => closed

Comment:

Thanks, but I couldn't reproduce with the provided commands, see
[https://dryorm.xterm.info/issue-37057 fiddle].
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:1>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: (none)
Type: Bug | Status: closed
Component: Database layer | Version: 6.0
(models, ORM) | Resolution:
Severity: Normal | worksforme
Keywords: UniqueConstraint | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Drews):

Replying to [comment:1 Jacob Walls]:

> Thanks, but I couldn't reproduce with the provided commands, see
[https://dryorm.xterm.info/issue-37057 fiddle].

My bad, I have created the entries via the admin site. Looks like the
UniqueConstraints aren't validated with `Device.objects.create()`.
See [https://dryorm.xterm.info/uqznomsn Updated fiddle] with
`full_clean()` call.
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:2>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: (none)

Type: Bug | Status: new

Component: Database layer | Version: 6.0
(models, ORM) |

Severity: Normal | Resolution:

Keywords: UniqueConstraint | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Drews):

* resolution: worksforme =>
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:3>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 6.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: UniqueConstraint | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Drews:

Old description:

New description:

register_b = Device(

name="Device B", pos_location=loc, cash_register_type=None
)

register_b.full_clean() # raises ValidationError

}}}

== Workaround
Add an explicit `isnull=False` check to the condition:
`condition=models.Q(cash_register_type=10) &
models.Q(cash_register_type__isnull=False)`
Although `10` should already be `isnull=False`.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:4>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: (none)
Type: Bug | Status: new

Component: Database layer | Version: 5.2

(models, ORM) |
Severity: Normal | Resolution:

Keywords: UniqueConstraint | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* stage: Unreviewed => Accepted
* version: 6.0 => 5.2

Comment:

Thanks, reproduced at a77be30a4346d111beaf366dcee4934734458a48. We seem to
have an issue with NULL handling in `UniqueConstraint.validate()` where
despite `nulls_distinct=None`, we still don't want `NULL = rhs` to
evaluate to `NULL` and cause the check's `COALESCE` to return true.
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:5>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------

Reporter: Drews | Owner: Simon
| Charette
Type: Bug | Status: assigned

Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: UniqueConstraint | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* owner: (none) => Simon Charette
* status: new => assigned

Comment:

I suspect we'll want to adjust the `nulls_distinct` check to ''resolve''
to what the backend actually defaults to if unspecified.
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:6>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: UniqueConstraint | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* has_patch: 0 => 1

Comment:

This involved changing when we `COALESCE` as the approach taken in #33996
was too generic. `nulls_distinct` shouldn't be a factor here as it only
affects the constraint itself (fields / expressions) and not the
`condition`'s handling of `NULL` / `UNKNOWN`.
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:7>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:

Keywords: UniqueConstraint | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:8>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:

Keywords: UniqueConstraint | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:9>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution:

Keywords: UniqueConstraint | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_better_patch: 1 => 0

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:10>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Drews | Owner: Simon
| Charette

Type: Bug | Status: closed

Component: Database layer | Version: 5.2
(models, ORM) |

Severity: Normal | Resolution: fixed

Keywords: UniqueConstraint | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"61a62be313e395ce1265132bfc99f51476fb3c95" 61a62be3]:
{{{#!CommitTicketReference repository=""
revision="61a62be313e395ce1265132bfc99f51476fb3c95"
Fixed #37057 -- Adjusted UniqueConstraint handling of UNKNOWN condition.

When we adjusted UNKNOWN handling for CheckConstraint in refs #33996 we
assumed
that all usage of Q.check would benefit from this approach.

However while CHECK constraints enforcement do ignore conditions involving
NULL
that resolve to UNKNOWN it's not the case for other type of constraints
such as
UNIQUE ones.

Given how UNKNOWN should be treated depends on the callers context it
appears
that a better strategy for COALESCE wrapping is to force them to apply it
if
necessary.

Thanks Drew Shapiro for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:11>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------

Reporter: Daniel Drews | Owner: Simon

| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: UniqueConstraint | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Daniel Drews):

Replying to [comment:11 Jacob Walls <jacobtylerwalls@…>]:

> Thanks Drew Shapiro for the report.

I'm not Drew Shapiro, by the way. I updated my name here. Just FYI.

And thank you all for your work!
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:12>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Daniel Drews | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: UniqueConstraint | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

I'm so sorry Daniel, I assumed you logged it with Github and that the
`Drews` Trac username was pointing to this [https://github.com/Drews
Github user].
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:13>

[Django]

#37057: UniqueConstraint incorrectly raises ValidationError on nullable fields in
condition
-------------------------------------+-------------------------------------
Reporter: Daniel Drews | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: UniqueConstraint | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Daniel Drews):

Replying to [comment:13 Simon Charette]:
> I'm so sorry Daniel, I assumed you logged in with Github and that the

`Drews` Trac username was pointing to this [https://github.com/Drews
Github user].

That's what I thought. Don't worry. All good.
--
Ticket URL: <https://code.djangoproject.com/ticket/37057#comment:14>