Opened 9 years ago

Closed 9 years ago

#2348 closed defect (fixed)

[patch] Unbound local error when performing invalid query

Reported by: cmgreen@… Owned by: adrian
Component: Database layer (models, ORM) Version: master
Severity: minor Keywords:
Cc: kmtracey@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When performing a model query with an invalid keyword, I get an UnboundLocalError rather than an error message saying it didn't know how to perform that type of query. I lost a bit of time trying to figure out how to debug this error but it was simplistic once I figured it out.

I used equals rather than exact

   One.objects.filter(name__equals=1) 
sw/lib/python2.4/site-packages/django/db/models/query.py in lookup_inner(path, lookup_type, value, opts, table, column)
    845     if path:
    846         # There are elements left in the path. More joins are required.
--> 847         if len(path) == 1 and path[0] in (new_opts.pk.name, None) \
    848             and lookup_type in ('exact', 'isnull') and not join_required:
    849             # If the next and final name query is for a primary key,

UnboundLocalError: local variable 'new_opts' referenced before assignment
Out[22]: 
In [23]: One.objects.filter(name__equals=1)                                  


Attachments (2)

query.py.diff (759 bytes) - added by Karen Tracey <kmtracey@…> 9 years ago.
patch_with_test.diff (1.4 KB) - added by Karen Tracey <kmtracey@…> 9 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 9 years ago by mtredinnick

This error won't ever be able to be reported as "invalid query type", because the lack of an invalid type just means we assume you have not specified one and append __exact to the end ("exact" is implied if nothing is given). The syntax you are using is valid for looking up the "equals" field in the related "name" model.

I suspect the actual error here is being caused by the same thing as #2287 (i.e. it's not clear why we are getting that far into the code). We can do a bit better in the error message, I suspect. This will need to wait until the current refactoring in django.db.model is completed, though.

comment:2 Changed 9 years ago by mtredinnick

(In [3490]) Seed the global app cache in a call to db.models.get_model() except when we are
constructing a model class. Refs #2348.

comment:3 Changed 9 years ago by mtredinnick

Whoops! [3490] does not ref this bug at all -- it refers to #2438.

Changed 9 years ago by Karen Tracey <kmtracey@…>

comment:4 Changed 9 years ago by Karen Tracey <kmtracey@…>

  • Cc kmtracey@… added
  • Summary changed from Unbound local error when performing invalid query to [patch] Unbound local error when performing invalid query

I ran into this also today. I was surprised to get:

UnboundLocalError: local variable 'new_opts' referenced before assignment

from File "/usr/lib/python2.4/site-packages/django/db/models/query.py", line 859,
in lookup_inner

when I mistyped a filter specification as Dateear instead of Dateyear.

Granted, it's a user error, but for most other filter typos I get something more along the lines of:

TypeError: Cannot resolve keyword 'xDate' into field

which clearly points me to where my typo is. UnboundLocalError looks likes a bug in the Django code itself.

I looked at the code in query.py and it's a simple fix to catch cases like this and raise the TypeError instead of continuing on and using variables that haven't been set. Is the refactoring mentioned above completed? I see query.py has been checked in quite recently so I am hoping the attached suggested patch could be useful in resolving this (and 2287, which looks to be the exact same problem).

comment:5 Changed 9 years ago by SmileyChris

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

Looks good, is there a way to write a test for this? It'd make it easier for a committer to ensure the behaviour is correct.

Changed 9 years ago by Karen Tracey <kmtracey@…>

comment:6 Changed 9 years ago by Karen Tracey <kmtracey@…>

  • Needs tests unset

OK, I took a shot at adding a test for this (first time doing anything with the tests but it ran OK when I gave it a try). Patch + test are in the new attachment.

comment:7 Changed 9 years ago by SmileyChris

  • Triage Stage changed from Accepted to Ready for checkin

Thanks, Karen -- well done on the patch + test. Usually we don't reference tickets, better to reference the problem directly in the comment, but a committer can fix that.

Good job and I look forwards to seeing some more patches from you ;)

comment:8 Changed 9 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [4470]) Fixed #2348 -- Improved error reporting when query filter arguments are
misspelt. Variation on a patch from Karen Tracey.

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