Opened 10 years ago
Closed 10 years ago
#24277 closed Cleanup/optimization (fixed)
Passing a dict with 2 keys to a queryset filter passes erroneously.
Reported by: | Keryn Knight | Owned by: | alexandrinaw |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | django@…, Nina Zakharenko, alexandrinaw | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
It's reasonably common to do this:
kws = {'first_name': 'test', 'is_superuser': False} get_user_model().objects.filter(**kws)
when building up a set of queryset filters, at least if you don't need to introduce ORs (via Q()
).
However, if you forget, or accidentally don't star-expand the dictionary:
get_user_model().objects.filter(kws)
You instead get back a queryset with the wrong SQL applied, specifically, above it yields WHERE first_name = 'is_superuser'
, though that may be a function of the relative stableness of dicts on Python2.
This is in contrast to if your kws
dict contains 1, or 3+ keys, which correctly error loudly and let you know you screwed up, yielding either:
ValueError: need more than 1 value to unpack ValueError: too many values to unpack (expected 2)
Or if you're lucky, and the get_prep_lookup
can figure out you've been silly:
# using kws = {'id': 1, 'is_superuser': False} ValueError: invalid literal for int() with base 10: 'is_superuser'
(This last is a relatively recent addition in master, as far as I can tell, having just pulled up to date from just after 1.8a I think)
The issue itself stems from Query.build_filter, and the specific line which causes the problem I think would survive the changes being made in #24267.
Change History (7)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 10 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 10 years ago
Cc: | added |
---|---|
Owner: | changed from | to
comment:4 by , 10 years ago
Has patch: | set |
---|
comment:6 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Agreed that it would be better to error out in this case.
Note that the behavior has been there forever (at least from 1.0 days, see https://github.com/django/django/blob/stable/1.0.x/django/db/models/sql/query.py#L1155)