[Django] #25718: Can’t use queries with JSON ’null’ values with JSONField
Django
----------------------------------+-----------------
Reporter: Stranger6667 | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
----------------------------------+-----------------
For regular fields it can be done with `is_null` lookup, but with JSON
field it is not an option.
Example setup.
{{{
from django.tests.postgres_tests.models import JSONModel
JSONModel.objects.create(field={‘key’: None})
JSONModel.objects.create(field={‘key’: 1})
}}}
Now lets get all objects where key is None:
{{{
>>> JSONModel.objects.filter(field__key__isnull=True)
}}}
Will fail with:
{{{
ProgrammingError: operator does not exist: jsonb -> boolean
LINE 1: ...onmodel" WHERE "postgres_tests_jsonmodel"."field" -> ‘key' IS
...
^
HINT: No operator matches the given name and argument type(s). You might
need to add explicit type casts.
}}}
Generated SQL:
{{{
SELECT "postgres_tests_jsonmodel"."id", "postgres_tests_jsonmodel"."field"
FROM "postgres_tests_jsonmodel" WHERE "postgres_tests_jsonmodel"."field"
-> ‘key' IS NULL;
}}}
Which is obviously incorrect.
Next try:
{{{
>>> JSONModel.objects.filter(field__key=None)
}}}
Will fail with:
{{{
ValueError: Cannot use None as a query value at
django.db.models.sql.query, line 985, in prepare_lookup_value
}}}
Will also fail:
{{{
JSONModel.objects.filter(field__key='null’)
}}}
Correct query should be:
{{{
SELECT "postgres_tests_jsonmodel"."id", "postgres_tests_jsonmodel"."field"
FROM "postgres_tests_jsonmodel" WHERE "postgres_tests_jsonmodel"."field"
-> ‘key' = 'null';
}}}
In my patch JSONField uses slightly modified lookups. Any thoughts? Is it
a good approach to solve this problem?
--
Ticket URL: <https://code.djangoproject.com/ticket/25718>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Stranger6667 | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
Changes (by Stranger6667):
* Attachment "json_null.patch" added.
Django
Reporter: Stranger6667 | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Stranger6667):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Changes is here:
https://github.com/django/django/compare/master...Stranger6667:fix-json-
isnull-lookups?expand=1
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:1>
Django
Reporter: Stranger6667 | Owner:
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by charettes):
* cc: charettes (added)
* stage: Unreviewed => Accepted
Comment:
Hi Stranger6667, thanks for the report and patch!
At a first glance you proposed changes make sense.
Please create a PR out of this branch and link back to it here as it will
give it more visibility and allows running CI against the iterations of
your patch.
Once this is done, detailing why this requires changing code outside the
`django.contrib.postgres` package would also help future reviewers.
Accepting based on the detailed report and the fact that we need an API to
query `null` values.
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:2>
Django
Reporter: Stranger6667 | Owner: Stranger6667
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Stranger6667):
* owner: => Stranger6667
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:3>
Django
Reporter: Stranger6667 | Owner: Stranger6667
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Stranger6667):
PR: https://github.com/django/django/pull/5617
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:4>
Django
Reporter: Stranger6667 | Owner: Stranger6667
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Stranger6667):
* version: 1.8 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:5>
Django
Reporter: Stranger6667 | Owner: Stranger6667
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Marking "Patch needs improvement" given Marc's feedback on the pull
request.
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:6>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/8239 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:7>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:8>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:9>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:10>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
For me, I think this is now good to go. (Not 100% sure about
`versionchanged` vs `versionadded` here...)
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:11>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:12>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
Release notes and docs have been updated to reflect changes. (Including BC
change if you were using `=None` to query for `__isnull`.) Should be good
to go.
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:13>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Type: Bug | Status: assigned
Component: contrib.postgres | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
Looking good, but I think `HStoreField` should have the same behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:14>
Django
Reporter: Dmitry Dygalo | Owner: Dmitry
| Dygalo
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c979c0a2b8abca325a549961fd7a17bdc36bcb1f" c979c0a]:
{{{
#!CommitTicketReference repository=""
revision="c979c0a2b8abca325a549961fd7a17bdc36bcb1f"
Fixed #25718 -- Made a JSONField lookup value of None match keys that have
a null value.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25718#comment:15>