Opened 13 years ago

Closed 13 years ago

#14766 closed Bug (duplicate)

ordering by a field that does not exists returns an empty QuerySet

Reported by: Rob Hudson Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords: sprintdec2010 order_by
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

If you .order_by('field_that_does_not_exist') and evaluate it you get an empty list. This can lead to bugs if you mistype the field name and wondering why you are getting no results. I would expect Django to raise an exception instead. It appears it the queryset doesn't even trigger a database call.

Attachments (2)

14766.diff (925 bytes ) - added by Rob Hudson 13 years ago.
14766_fix_with_tests.diff (2.1 KB ) - added by Aram Dulyan 13 years ago.
Fix with tests (derived from patch above)

Download all attachments as: .zip

Change History (16)

comment:1 by Matthew Schinckel, 13 years ago

Hmm. I get:

>>> Person.objects.order_by('foo')
[…traceback…]
FieldError: Cannot resolve keyword 'foo' into field. Choices are: […]

This is in both django 1.2 and the latest trunk.

comment:2 by Rob Hudson, 13 years ago

Yeah, hmm...

I just tested on another project and got the same result on both my own models and auth.User. This is Django 1.2 and trunk using psycopg2 backend. I'll try to dig further to see what could be causing this if I can.

in reply to:  2 comment:3 by Rob Hudson, 13 years ago

Replying to robhudson:

Yeah, hmm...

I just tested on another project and got the same result on both my own models and auth.User. This is Django 1.2 and trunk using psycopg2 backend. I'll try to dig further to see what could be causing this if I can.

By same result I mean the empty queryset.

comment:4 by Chris Beaven, 13 years ago

Resolution: worksforme
Status: newclosed

FieldError (as expected) for me too (tried one project on trunk and psycopg2 and another on 1.2 and sqlite)

Marking worksforme, but feel free to reopen when more info comes to light.

comment:5 by Dougal Matthews, 13 years ago

Resolution: worksforme
Status: closedreopened

FWIW after speaking to Rob on irc I tried and I can reproduce with this. Although, interestingly adding a count at the end makes the query "work."

In [5]: import django

In [6]: django.VERSION
Out[6]: (1, 2, 3, 'final', 0)

In [7]: from django.contrib.contenttypes.models import ContentType

In [8]: ContentType.objects.order_by('monkeys')
Out[8]: []

In [9]: ContentType.objects.order_by('monkeys').count()
Out[9]: 20

Using the mysql backend. I also tried on 1.1.1 with the same results (albeit a different project).

comment:6 by Rob Hudson, 13 years ago

I searched the current Django tests and didn't see anything explicitly testing for an assertRaises on an invalid field when using order_by. I added one in regressiontests/model_fields and ran it and it fails to raise the FieldError.

comment:7 by Chris Beaven, 13 years ago

Which, I'm assume, you're about to attach? :)

by Rob Hudson, 13 years ago

Attachment: 14766.diff added

comment:8 by Aram Dulyan, 13 years ago

Keywords: sprintdec2010 added
Triage Stage: UnreviewedAccepted

The actual issue appears to be that the validation of the arguments to order_by doesn't occur until the query is about to be evaluated. So q = Model.objects.order_by('monkeys') doesn't raise a FieldError, but list(Model.objects.order_by('monkeys')) does. However, the validation occurs before the query is passed to the database, which is comforting.

d0ugal: it makes sense for count() to work, since order_by is ignored completely when getting a count.

by Aram Dulyan, 13 years ago

Attachment: 14766_fix_with_tests.diff added

Fix with tests (derived from patch above)

comment:9 by Aram Dulyan, 13 years ago

Has patch: set
Patch needs improvement: set

I added a patch, but I'm not happy with its DRYness. All tests in the suite pass. I'll leave it to someone else to improve upon.

comment:10 by Russell Keith-Magee, 13 years ago

Resolution: wontfix
Status: reopenedclosed

The problem here is being caused by Python itself. For a couple of releases of Python 2.6, http://bugs.python.org/issue1242657 meant that exceptions raised during iteration were transparently swallowed by the iteration process. The field error *is* being raised; but the fact that you're iterating over the results means that the exception is eaten, resulting in an empty result set.

If you're on a Mac with a default Snow Leopard Python install, you have Python 2.6.1, which is affected. If you have an up to date Python install (2.6.5 at time of writing), the problem has been fixed. I *think* it was fixed in Python 2.6.2, but I haven't got an install handy to check.

Since this appears to be due to a bug in Python that has been addressed, and it's a non-trivial thing to work around, I'm going to mark this wontfix. If you are affected by this, the solution is to upgrade to Python 2.6.5+.

comment:11 by Aram Dulyan, 13 years ago

Resolution: wontfix
Status: closedreopened

I was able to replicate the issue with Python 2.7.1 (MacPorts build on Snow Leopard). Reopening until I (or someone else) run some tests with different Python versions to confirm.

comment:12 by Karen Tracey, 13 years ago

I don't have a Mac, but I cannot recreate the original problem with the Python 2.7.1 I have available to me (Windows):

Python 2.7.1 (r271:86832, Nov 27 2010, 18:30:46) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django.contrib.contenttypes.models import ContentType
>>> ContentType.objects.order_by('monkeys')
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "C:\u\kmt\django\trunk\django\db\models\query.py", line 69, in __repr__
    data = list(self[:REPR_OUTPUT_SIZE + 1])
  File "C:\u\kmt\django\trunk\django\db\models\query.py", line 84, in __len__
    self._result_cache.extend(list(self._iter))
  File "C:\u\kmt\django\trunk\django\db\models\query.py", line 273, in iterator
    for row in compiler.results_iter():
  File "C:\u\kmt\django\trunk\django\db\models\sql\compiler.py", line 680, in results_iter
    for rows in self.execute_sql(MULTI):
  File "C:\u\kmt\django\trunk\django\db\models\sql\compiler.py", line 725, in execute_sql
    sql, params = self.as_sql()
  File "C:\u\kmt\django\trunk\django\db\models\sql\compiler.py", line 60, in as_sql
    ordering, ordering_group_by = self.get_ordering()
  File "C:\u\kmt\django\trunk\django\db\models\sql\compiler.py", line 349, in get_ordering
    self.query.model._meta, default_order=asc):
  File "C:\u\kmt\django\trunk\django\db\models\sql\compiler.py", line 378, in find_ordering_name
    opts, alias, False)
  File "C:\u\kmt\django\trunk\django\db\models\sql\query.py", line 1215, in setup_joins
    "Choices are: %s" % (name, ", ".join(names)))
FieldError: Cannot resolve keyword 'monkeys' into field. Choices are: app_label, id, logentry, model, name, permission

So it does not look like the problem re-appeared in Python 2.7 generally, and it does not seem like the kind of bug that would appear in just one distro, so I'm confused by the report that it exists in the Mac version of 2.7.1.

(Some history of Django's encounters with this Python bug, and a link to the Python bug, can be found in #7786. In that ticket Malcolm indicated it would be quite difficult to attempt to avoid raising any exceptions in __len__, and the "fix" for the test that was failing as a result was just to skip it on Pythons known to have the problem.)

comment:13 by anonymous, 13 years ago

Severity: Normal
Type: Bug

comment:14 by Jacob, 13 years ago

Resolution: duplicate
Status: reopenedclosed

I've opened #15867 to track the need to document this issue.

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