-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
Fixed #36508 -- Interpreted __iexact=None on KeyTransforms as __exact=None. #19793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
buildbot, test on oracle. |
…=None. Thanks Jacob Walls for the report.
4886931
to
48ce520
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This looks good to me 👍
# Interpret '__exact=None' as the SQL 'IS NULL'. For '__iexact=None' on | ||
# KeyTransform, interpret it as '__exact=None' instead of 'IS NULL' | ||
# (#36508). For all other cases, reject the use of None as a query | ||
# value unless the lookup explicitly supports it. | ||
if lookup.rhs is None and not lookup.can_use_none_as_rhs: | ||
if lookup_name not in ("exact", "iexact"): | ||
raise ValueError("Cannot use None as a query value") | ||
if lookup_name == "iexact" and isinstance(lhs, KeyTransform): | ||
return lhs.get_lookup("exact")(lhs, None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having had another think, it would be preferable if we don't add json specific logic here
Can we not update django.db.models.fields.json.KeyTransformIExact
somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I share your sentiment @sarahboyce, this should be implemented at the KeyTransformIExact
level and not hardcoded here. The reason why exact
and iexact
are here today is due to how they must span OUTER JOIN
when targeting a reverse relationship, I hope to remove it entirely with #16817.
Trac ticket number
ticket-36508
Branch description
Matches expectations by reserving the use of
None
in key transform lookups for JSON null.Checklist
main
branch.