[Django] #31894: JSONField filter() and JSONField exclude() are sometimes not inverses of each other
Django
each other
-----------------------------------------+------------------------
Reporter: Mikuael | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 3.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
Filtering based on a JSONField key-value pair seems to have some
unexpected behavior when involving a key that not all records have.
Strangely, filtering on an optional property key will not return the
inverse result set that an exclude on the same property key will return.
In my database, I have:
* 2250 total records
* 49 records where jsonfieldname = {'propertykey': 'PropertyValue'}
* 296 records where jsonfieldname has a 'propertykey' key with some other
value
* 1905 records where jsonfieldname does not have a 'propertykey' key at
all
The following code:
{{{
q = Q(jsonfieldname__propertykey="PropertyValue")
total_records = Record.objects.count()
filtered_records = Record.objects.filter(q).count()
excluded_records = Record.objects.exclude(q).count()
filtered_plus_excluded_records = filtered_records + excluded_records
print('Total: %d' % total_records)
print('Filtered: %d' % filtered_records)
print('Excluded: %d' % excluded_records)
print('Filtered Plus Excluded: %d' % filtered_plus_excluded_records)
}}}
Will output this:
{{{
Total: 2250
Filtered: 49
Excluded: 296
Filtered Plus Excluded: 345
}}}
It is surprising that the filtered+excluded value is not equal to the
total record count. It's surprising that the union of a expression plus
its inverse does not equal the sum of all records. I am not aware of any
other queries in Django that would return a result like this. I realize
adding a check that the key exists would return a more expected results,
but that doesn't stop the above from being surprising.
I'm not sure what a solution would be - either a note in the documentation
that this behavior should be expected, or take a look at how this same
expression is applied for both the exclude() and filter() queries and see
why they are not opposites.
--
Ticket URL: <https://code.djangoproject.com/ticket/31894>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
each other
Reporter: Mikuael | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 3.1
Severity: Normal | 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 Carlton Gibson):
* type: Uncategorized => Cleanup/optimization
* component: Uncategorized => Documentation
* stage: Unreviewed => Accepted
Comment:
Hi Mikuael. Thanks for the report. Yes, let's take this as a Documentation
issue: it's expected behaviour but worth calling out explicitly.
(If you wanted to add a few additional test cases for `exclude` that might
be cool too, documenting and verifying behaviour, on each of the backend
DBs.)
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:1>
Django
each other
Reporter: Mikuael | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by sage):
* cc: sage (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:2>
Django
each other
Reporter: Mikuael | Owner: jpribyl
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jpribyl):
* cc: jpribyl (added)
* owner: nobody => jpribyl
* status: new => assigned
Comment:
I can look at this one! It looks pretty straightforward.
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:3>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by sage):
I think we can change the existing Oracle note about `exclude()`
([https://docs.djangoproject.com/en/3.1/topics/db/queries/#key-index-and-
path-transforms here]) to make it more general (not just with `None` rhs)
and remove the `Oracle users` note title to just `Note` or `Warning`. Like
what Carlton said, it would be great if we also add test cases for that. I
think we should also explicitly say that in order to get the proper
inverse of `filter()`, users should also consider querying with
`__isnull`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:4>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Johnny Pribyl):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/13367
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:5>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Johnny Pribyl):
Sage, sorry just now saw your comment! I wound up adding a new section but
am happy to combine the two if you think that makes more sense.
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:6>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Johnny Pribyl):
Looks like there are some changes to be made - I didn't realize quite how
dependent this behavior was on db backend! I will have to look at this a
bit more. Do you guys know if there is a way to specify db backend when
running tests locally?
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:7>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:8>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:9>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"94e22381079d65dccb9ac1ad9a9ebea2af2fdf66" 94e22381]:
{{{
#!CommitTicketReference repository=""
revision="94e22381079d65dccb9ac1ad9a9ebea2af2fdf66"
Refs #31894 -- Added tests for JSONField key lookups with
QuerySet.exclude().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:10>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Component: Documentation | Version: 3.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"17407eca59b0572228067cdee51433f49a1e0adb" 17407eca]:
{{{
#!CommitTicketReference repository=""
revision="17407eca59b0572228067cdee51433f49a1e0adb"
Fixed #31894 -- Added note about using JSONField key lookups with
QuerySet.exclude() in docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:11>
Django
each other
Reporter: Mikuael | Owner: Johnny
Type: | Pribyl
Cleanup/optimization | Status: closed
Component: Documentation | Version: 3.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"3fc636244e07c02645fcebda769d7adf4ab560a6" 3fc63624]:
{{{
#!CommitTicketReference repository=""
revision="3fc636244e07c02645fcebda769d7adf4ab560a6"
[3.1.x] Fixed #31894 -- Added note about using JSONField key lookups with
QuerySet.exclude() in docs.
Backport of 17407eca59b0572228067cdee51433f49a1e0adb from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31894#comment:12>