[Django] #34754: CheckConstraint with isnull lookup on JSONField transform None into null jsonb value
Django
jsonb value
-------------------------------------+-------------------------------------
Reporter: Alexandre | Owner: nobody
Collet |
Type: | Status: new
Uncategorized |
Component: Database | Version: 4.2
layer (models, ORM) |
Severity: Normal | Keywords: constraint,
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Hello !
I think i've found a bug in the validation of constraints for the
JSONField with the isnull lookup (with postgresql).
Let me explain:
I have a model with some values stored in a JSONField and i want to be
sure (with a CheckConstraint) than if this field was not None, an author
was set.
{{{
class MyModel(Model, UuidMixin, TimestampableMixin):
values = JSONField(
null=True,
blank=True,
)
author = ForeignKey(
null=True,
blank=True,
to=settings.AUTH_USER_MODEL,
on_delete=SET_NULL,
)
class Meta:
constraints = [
CheckConstraint(
name="author_cannot_be_null_if_values_was_set",
check=(
Q(values__isnull=True)
| Q(values__isnull=False, author__isnull=False)
),
),
]
}}}
This works perfectly in 4.1.
But in 4.2, the sql generated for the check when i does not set the field
"values" and "author" is that:
{{{
instance = MyModel(values=None, author=None)
instance.validate_constraints()
}}}
{{{
SELECT 1 AS "_check" WHERE COALESCE((('null'::jsonb IS NULL OR (NULL IS
NOT NULL AND 'null'::jsonb IS NOT NULL))), true)
}}}
This raise a ValidationError because the None value was transformed into a
jsonb null value and the isnull lookup is always False.
I'm not sure how to correct this, but if anyone would like to help me, I'd
be delighted to make a pull request.
--
Ticket URL: <https://code.djangoproject.com/ticket/34754>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
jsonb value
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: constraint, | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:1>
Django
jsonb value
-------------------------------------+-------------------------------------
Reporter: Alexandre Collet | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: constraint, | Triage Stage:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Alexandre Collet):
* keywords: constraint, => constraint, jsonfield, none, check
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:2>
Django
jsonb value
-------------------------------------+-------------------------------------
Reporter: Alexandre Collet | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Keywords: constraint, | Triage Stage: Accepted
jsonfield, none, check |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report. Regression in
5c23d9f0c32f166c81ecb6f3f01d5077a6084318.
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:3>
Django
jsonb value
-------------------------------------+-------------------------------------
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: constraint, | Triage Stage: Accepted
jsonfield, none, check |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Simon Charette
* status: new => assigned
* has_patch: 0 => 1
Comment:
Submitted a patch to address the issue but I think the fundamental problem
here is that
{{{#!python
MyModel.objects.create(values=None) # Creates an object with a SQL NULL
MyModel.objects.filter(values=None) # Filters for objects with JSON NULL
}}}
This asymmetry prevents logic from being added to
`JSONField.get_db_prep_value` directly but it's a much larger problem to
resolve.
Just like [https://docs.djangoproject.com/en/4.2/ref/models/fields/#null
we discourage] the usage of `null=True, blank=True` fields as allows for
the co-existence of two ''no data'' values I think we should do add
''friction'' to storing `null` in a `JSONField(null=True)` field by
requiring that `Value(None, JSONField())` be used to filter using
`'null'::json`.
I can't think of a deprecation path for
`MyModel.objects.filter(values=None)` though unfortunately so it seems
like we've missed the boat on that one.
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:4>
Django
jsonb value
-------------------------------------+-------------------------------------
Reporter: Alexandre Collet | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
jsonfield, none, check | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:5>
Django
jsonb value
-------------------------------------+-------------------------------------
Reporter: Alexandre Collet | Owner: Simon
| Charette
Component: Database layer | Version: 4.2
(models, ORM) |
Keywords: constraint, | Triage Stage: Ready for
jsonfield, none, check | 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:"3434dbd39d373df7193ad006b970c09c1a909ea3" 3434dbd3]:
{{{
#!CommitTicketReference repository=""
revision="3434dbd39d373df7193ad006b970c09c1a909ea3"
Fixed #34754 -- Fixed JSONField check constraints validation on NULL
values.
The __isnull lookup of JSONField must special case
Value(None, JSONField()) left-hand-side in order to be coherent with
its convoluted null handling.
Since psycopg>=3 offers no way to pass a NULL::jsonb the issue is
resolved by optimizing IsNull(Value(None), True | False) to
True | False.
Regression in 5c23d9f0c32f166c81ecb6f3f01d5077a6084318.
Thanks Alexandre Collet for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:6>
Django
jsonb value
-------------------------------------+-------------------------------------
Reporter: Alexandre Collet | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: constraint, | Triage Stage: Ready for
jsonfield, none, check | 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:"3a1863319c1ed4bc81fe270b0d7d25c7f03fdb48" 3a186331]:
{{{
#!CommitTicketReference repository=""
revision="3a1863319c1ed4bc81fe270b0d7d25c7f03fdb48"
[4.2.x] Fixed #34754 -- Fixed JSONField check constraints validation on
NULL values.
The __isnull lookup of JSONField must special case
Value(None, JSONField()) left-hand-side in order to be coherent with
its convoluted null handling.
Since psycopg>=3 offers no way to pass a NULL::jsonb the issue is
resolved by optimizing IsNull(Value(None), True | False) to
True | False.
Regression in 5c23d9f0c32f166c81ecb6f3f01d5077a6084318.
Thanks Alexandre Collet for the report.
Backport of 3434dbd39d373df7193ad006b970c09c1a909ea3 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:7>
Django
jsonb value
-------------------------------------+-------------------------------------
Reporter: Alexandre Collet | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: constraint, | Triage Stage: Ready for
jsonfield, none, check | 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:"936afc2deb7e20f87c117d25a96e285384fc77dd" 936afc2d]:
{{{
#!CommitTicketReference repository=""
revision="936afc2deb7e20f87c117d25a96e285384fc77dd"
[4.2.x] Refs #34754 -- Added missing FullResultSet import.
Follow up to 3a1863319c1ed4bc81fe270b0d7d25c7f03fdb48.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34754#comment:8>