Re: [Django] #33996: Inconsistent behavior of CheckConstraints validation on None values.
Django
-------------------------------------+-------------------------------------
Reporter: James Beith | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by David Sanders):
* cc: David Sanders (added)
Comment:
Aha I'd also just noticed this behaviour.
It's due to the check constraint doing the validation in the WHERE clause
of a query… a NULL value producing an empty resultset hence failing the
validation:
{{{
SELECT 1 AS "_check" WHERE NULL >= 0
}}}
The original PR for this feature mentions moving to WHERE from SELECT to
get around certain issue – which is somewhat unfortunate because using
SELECT could've given the opportunity to check the result against false.
My naive suggestion would be to add an |Q(field=None) against nullable
fields in the check's query.
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: James Beith | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
Another alternative would be to coalesce the filters of the query (or the
entire WHERE clause) which might be the simpler choice:
{{{
postgres=# SELECT 1 WHERE coalesce((NULL >= 18), 't');
?column?
----------
1
(1 row)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: James Beith | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Sanders):
Not to take away from any recommendation from Gagaro or Simon Charette,
but here's a patch + test demonstrating coalesce solving the issue:
https://github.com/django/django/compare/main...shangxiao:django:ticket_33996
Note though this pushes logic back into the SELECT and I haven't an Oracle
installation to test with. Solution also naively ignores non-nullable
fields.
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: James Beith | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
Comment:
Thanks for the report and PR folks, this was definitely overlooked during
this feature's development.
I went through the comments on the PR that introduced the feature and
[https://github.com/django/django/pull/14625#pullrequestreview-911482741
it seems we could likely work around the Oracle limitation] by using
`SELECT CASE WHEN` by having the wrapping happen directly in `Q.check`
instead of trying to be clever about relocating the
`select_format`/`supports_boolean_expr_in_select_clause` as originally
attempted. Alternatively
As for non-nullable fields I don't think that it should be the
responsibility of `Constraint.validate` to account for that, we already
got validation logic in place to make sure that `null=False` don't allow
`NULL` and having all constraints associated with a non-nullable field
that are validated against a `None` value for this field raise the same
precondition failed `ValidationError` seems confusing.
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:5>
Django
-------------------------------------+-------------------------------------
| Sanders
Type: Bug | Status: assigned
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => David Sanders
* status: new => assigned
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: James Beith | Owner: David
| Sanders
Component: Database layer | Version: 4.1
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e14d08cd894e9d91cb5d9f44ba7532c1a223f458" e14d08cd]:
{{{
#!CommitTicketReference repository=""
revision="e14d08cd894e9d91cb5d9f44ba7532c1a223f458"
Fixed #33996 -- Fixed CheckConstraint validation on NULL values.
Bug in 667105877e6723c6985399803a364848891513cc.
Thanks James Beith for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: James Beith | Owner: David
| Sanders
Type: Bug | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | 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 Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"be5e3b46f78d54f8fae5dc255295d771c2becaac" be5e3b46]:
{{{
#!CommitTicketReference repository=""
revision="be5e3b46f78d54f8fae5dc255295d771c2becaac"
[4.1.x] Fixed #33996 -- Fixed CheckConstraint validation on NULL values.
Bug in 667105877e6723c6985399803a364848891513cc.
Thanks James Beith for the report.
Backport of e14d08cd894e9d91cb5d9f44ba7532c1a223f458 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: James Beith | Owner: David
| Sanders
Type: Bug | Status: closed
Component: Database layer | Version: 4.1
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In [changeset:"36a000858b5eee13b039aaad490c33b037a1dc05" 36a00085]:
{{{#!CommitTicketReference repository=""
revision="36a000858b5eee13b039aaad490c33b037a1dc05"
Refs #33996 -- Updated CheckConstraint validation on NULL values on Oracle
23c+.
Oracle 23c supports comparing boolean expressions.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33996#comment:9>