#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 , 4 years ago
Cc: | added |
---|---|
Component: | Uncategorized → Database layer (models, ORM) |
comment:2 by , 4 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
comment:3 by , 4 years ago
Type: | Uncategorized → New feature |
---|
follow-up: 5 comment:4 by , 4 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 , 4 years ago
When a key transform returns a JSONField
you can use any of these lookups or transforms, see tests.
comment:6 by , 4 years ago
follow-up: 8 comment:7 by , 4 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 , 4 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_key
works on all levels, you can usefilter(field__a__has_key='b')
.Duplicate of #31324 and #31894.