Opened 2 months ago

Last modified 3 weeks ago

#36508 assigned Bug

Asymmetry between exact and iexact when filtering JSON keys against None

Reported by: Jacob Walls Owned by: Clifford Gama
Component: Database layer (models, ORM) Version: 5.2
Severity: Normal Keywords: null, jsonfield
Cc: Sage Abdullah Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Jacob Walls)

exact and iexact vary in their treatment of None when looking up JSON key paths:

optional_param = request.GET.get("param", None)
qs1 = Model.objects.filter(json__key=optional_param)
qs2 = Model.objects.filter(json__key__iexact=optional_param)

fiddle showing exact queries return rows where the json key is null, but iexact queries do not (instead, they check for key existence, e.g. __key__isnull)

Previous ticket regarding symmetry between exact and iexact: #21552
Previous ticket regarding iexact on JSON key lookups: #27693

Change History (11)

comment:1 by Jacob Walls, 2 months ago

Description: modified (diff)

comment:2 by Simon Charette, 2 months ago

I feel like this falls into the same bucket, or at least very close, to the other issues relating to trying to do the right thing when None is provided to JSONField and that the solution lies in making None always translate to NULL and require JSONNull in cases where JSON null is required.

comment:3 by Natalia Bidart, 2 months ago

Resolution: duplicate
Status: newclosed

I agree with you, Simon, closing this as a duplicate of #35381. Jacob, just to double check: do you see any reason why fixing #35381 wouldn't also resolve this ticket?

comment:4 by Jacob Walls, 2 months ago

Sounds right. Catching up with the comments on #35381 I see that the plan is to change the behavior of json__key=None to match json__key__isnull=True, so that should bring exact into alignment with iexact which already does this. Thanks all!

comment:5 by Sage Abdullah, 3 weeks ago

Cc: Sage Abdullah added
Resolution: duplicate
Status: closednew

With the discussion in #35381, I think this is a separate issue. The main reason behind adding a JSONNull() that resulted in the creation of that ticket is for addressing the ambiguity of top-level values. Even if #35381 also adds support for using json__key=JSONNull() (at least in the querying case), it's clear that we do want to keep json__key=None to match JSON null in both querying and storing cases.

If #35381 is addressed, I imagine the code that handles None in JSONExact will be replaced with a JSONNull check. Then, the can_use_none_as_rhs probably needs to be moved from JSONExact to KeyTransformExact.

However, it's likely KeyTransformIExact will need the same treatment, as it currently only extends CaseInsensitiveMixin, KeyTransformTextLookupMixin, lookups.IExact without additional code. We might need to refactor KeyTransformExact so that some of its code can be reused by KeyTransformIExact. From a quick guess, these changes seem isolated to #35381.

comment:6 by Jacob Walls, 3 weeks ago

Triage Stage: UnreviewedAccepted

comment:7 by Clifford Gama, 3 weeks ago

A note that this ticket should be specifically for key transform lookups __key__iexact and __key__exact=None, not on the top-level, as the latter will be solved by #35381

Version 0, edited 3 weeks ago by Clifford Gama (next)

comment:8 by Clifford Gama, 3 weeks ago

Owner: set to Clifford Gama
Status: newassigned

comment:9 by Clifford Gama, 3 weeks ago

If I understand correctly, the plan is:

  1. Deprecate json__key__iexact=None (currently meaning __json__key__isnull=True), and point users to use __isnull=True instead.
  2. After the deprecation period, change __json__key__iexact=None to match JSON null.

comment:10 by Jacob Walls, 3 weeks ago

We might want to just fix without a deprecation(*), since there is no "upgrade path". If we warn about this, and you discover you really wanted the "new" behavior all along, the only answer would be to just wait, which isn't great.

(*) but doc it as a minor incompatible change?

I feel like most who have gone to the trouble to specify iexact wouldn't expect iexact to simply check key existence, and thus will want the "new" behavior.

comment:11 by Clifford Gama, 3 weeks ago

Has patch: set
Note: See TracTickets for help on using tickets.
Back to Top