#32237 closed New feature (duplicate)
JSONField isnull and =None
| Reported by: | Gordon Wrigley | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 3.1 |
| Severity: | Normal | Keywords: | |
| Cc: | Adam Johnson | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
JSONFields are as far as I know the only place in the system where Q(field__subfield__isnull=True) and Q(field__subfield=None) are not synonyms.
This also creates a situation where filter and exclude are not complements, which as far as I know is also unique to JSONFields.
And finally it creates a situation where MyModel.objects.filter(field__subfield__isnull=False).values(field__subfield) may include None.
All three of these violate things I understood to be invariants of the ORM. Additionally it is not obvious at the call site that non standard logic is in effect. You have to remember that field is a JSONField and that JSONFields treat None / null differently from other fields.
I think I understand the logic of this and how to some extent it falls naturally out of the implementation, but having worked with it a bunch now I think it creates a dangerous trap for the unwary and unexperienced.
I think isnull and =None on JSONFields should be changed so both match either an actual JSON null value or a missing field.
And either the has_key lookup should be extended to work at all levels so it can be used to distinguish between these cases with filter(field__has_key="a__b").
Or a new lookup should be added, (for the sake of discussion key_exists) that does what the current isnull lookup is doing so you can distinguish with filter(field__a__b__key_exists=True).
For the sake of discussion I've enumerated the current behaviour below.
With regard to this code [o.field for o in MyModel.objects.all()] we get:
0 all() [{}, {'subfield': None}, {'subfield': 123}]
11 filter(field__subfield=None) [{'subfield': None}]
12 filter(field__subfield__isnull=True) [{}]
13 filter(field__subfield__isnull=False) [{'subfield': None}, {'subfield': 123}]
14 exclude(field__subfield=None) [{'subfield': 123}]
15 exclude(field__subfield__isnull=True) [{'subfield': None}, {'subfield': 123}]
16 exclude(field__subfield__isnull=False) [{}]
21 filter(Q(field__subfield=None)) [{'subfield': None}]
22 filter(Q(field__subfield__isnull=True)) [{}]
23 filter(Q(field__subfield__isnull=False)) [{'subfield': None}, {'subfield': 123}]
24 exclude(Q(field__subfield=None)) [{'subfield': 123}]
25 exclude(Q(field__subfield__isnull=True)) [{'subfield': None}, {'subfield': 123}]
26 exclude(Q(field__subfield__isnull=False)) [{}]
31 filter(~Q(field__subfield=None)) [{'subfield': 123}]
32 filter(~Q(field__subfield__isnull=True)) [{'subfield': None}, {'subfield': 123}]
33 filter(~Q(field__subfield__isnull=False)) [{}]
34 exclude(~Q(field__subfield=None)) [{'subfield': None}]
35 exclude(~Q(field__subfield__isnull=True)) [{}]
36 exclude(~Q(field__subfield__isnull=False)) [{'subfield': None}, {'subfield': 123}]
Things to note:
The 2* lines behave the same as the 1* lines and can be ignored hence forth.
The *1, *4 pairs are not complements, by which I mean they do not between them return all rows.
The *1, *2 pairs and the *4, *5 pairs are not synonyms.
Change History (8)
comment:1 by , 5 years ago
| Cc: | added |
|---|---|
| Component: | Uncategorized → Database layer (models, ORM) |
comment:2 by , 5 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | new → closed |
comment:3 by , 5 years ago
| Type: | Uncategorized → New feature |
|---|
follow-up: 5 comment:4 by , 5 years ago
has_key works on all levels, you can use filter(fieldahas_key='b').
That's interesting, I will need to go test that, it's not what the docs say.
"Returns objects where the given key is in the top-level of the data."
https://docs.djangoproject.com/en/3.1/topics/db/queries/#has-key
comment:5 by , 5 years ago
When a key transform returns a JSONField you can use any of these lookups or transforms, see tests.
comment:6 by , 5 years ago
follow-up: 8 comment:7 by , 5 years ago
Thanks for this ticket, however it was already discussed and it's a documented behavior
I appreciate that it's documented behaviour, what I'm saying is the documented behaviour is bad in the surprising and dangerous sense and I think it should be improved.
Do I need to raise this to the developers mailing list?
comment:8 by , 5 years ago
Replying to Gordon Wrigley:
Thanks for this ticket, however it was already discussed and it's a documented behavior
I appreciate that it's documented behaviour, what I'm saying is the documented behaviour is bad in the surprising and dangerous sense and I think it should be improved.
Do I need to raise this to the developers mailing list?
Yes, you can start a discussion on DevelopersMailingList if you don't agree. Please refer to the original ticket and take into account that we need to reach a strong consensus on the mailing list to make a backward incompatible change in a documented behavior. It will be good to start from summarizing arguments mentioned in the original ticket and PR.
Personally, I believe that the current behavior is correct.
Thanks for this ticket, however it was already discussed and it's a documented behavior, see Storing and querying for None and Key, index, and path transforms docs.
__has_keyworks on all levels, you can usefilter(field__a__has_key='b').Duplicate of #31324 and #31894.