Code

Opened 3 years ago

Closed 3 years ago

#14766 closed Bug (duplicate)

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

Reported by: robhudson 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: UI/UX:

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 robhudson 3 years ago.
14766_fix_with_tests.diff (2.1 KB) - added by Aramgutang 3 years ago.
Fix with tests (derived from patch above)

Download all attachments as: .zip

Change History (16)

comment:1 Changed 3 years ago by schinckel

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 follow-up: Changed 3 years ago by 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.

comment:3 in reply to: ↑ 2 Changed 3 years ago by robhudson

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 Changed 3 years ago by SmileyChris

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

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 Changed 3 years ago by d0ugal

  • Resolution worksforme deleted
  • Status changed from closed to reopened

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 Changed 3 years ago by robhudson

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 Changed 3 years ago by SmileyChris

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

Changed 3 years ago by robhudson

comment:8 Changed 3 years ago by Aramgutang

  • Keywords sprintdec2010 added
  • Triage Stage changed from Unreviewed to Accepted

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.

Changed 3 years ago by Aramgutang

Fix with tests (derived from patch above)

comment:9 Changed 3 years ago by Aramgutang

  • 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 Changed 3 years ago by russellm

  • Resolution set to wontfix
  • Status changed from reopened to closed

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 Changed 3 years ago by Aramgutang

  • Resolution wontfix deleted
  • Status changed from closed to reopened

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 Changed 3 years ago by kmtracey

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 Changed 3 years ago by anonymous

  • Severity set to Normal
  • Type set to Bug

comment:14 Changed 3 years ago by jacob

  • Resolution set to duplicate
  • Status changed from reopened to closed

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.