Opened 18 years ago
Closed 18 years ago
#2210 closed defect (duplicate)
[patch] order_by on related table with db_column different to name fails
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | normal | Keywords: | order_by related |
Cc: | bs1984@…, russell@…, gary.wilson@…, real.human@…, arthur.case@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi,
When ordering by a related table which has a db_column different to the field name, and ordering by the field name, incorrect SQL is generated.
This bug is on line 483 of django/db/models/query.py (r3189) -- it incorrectly uses the opts of the main table, not the related table in the call to orderfield2column(). This shows up in the admin interface, when sorting by a related field.
I do not know the correct fix for this bug.
Russell
Attachments (4)
Change History (27)
comment:1 by , 18 years ago
Cc: | added |
---|
comment:2 by , 18 years ago
comment:3 by , 18 years ago
This is a different bug to #2076, fwict. The joins are being correctly generated in my case, but the wrong column name is being used.
comment:4 by , 18 years ago
Cc: | added; removed |
---|
comment:5 by , 18 years ago
Summary: | order_by on related table with db_column different to name fails → [patch] order_by on related table with db_column different to name fails |
---|
I added a patch that allows you to do order by related fields.
Foo.objects.all().order_by('name') Bar.objects.order_by('-foo__name') Baz.objects.order_by('bar__foo__name')
The Admin interface needs to be updated to make use of this.
comment:6 by , 18 years ago
Cc: | added |
---|
comment:7 by , 18 years ago
Cc: | added |
---|
comment:8 by , 18 years ago
Cc: | added |
---|
comment:9 by , 18 years ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
The orignal patch failed to support order_by('?'), though it tried to handle it twice. I've revised it, and now it passes all tests. It still needs additional tests to demonstrate that order_by('xxx_yyy') works now. Thanks, serbaut, for the original patch, I didn't really have to do a lot ;-)
comment:10 by , 18 years ago
comment:11 by , 18 years ago
Keywords: | order_by related added |
---|
This bug is confirmed. Here's a complete example to reproduce the problem:
models.py
from django.db import models class Book(models.Model): title = models.TextField() class BookStat(models.Model): book = models.OneToOneField(Book, related_name='stats') times_read = models.PositiveIntegerField()
After syncing the DB, in the command line:
>>> from orderby.example.models import * >>> book = Book.objects.create(title="BFG") >>> book.save() >>> bs = BookStat.objects.create(book=book, times_read=15) >>> bs.save() >>> Book.objects.all().order_by("stats__times_read") Traceback (most recent call last): File "<console>", line 1, in <module> File "C:\Python25\lib\site-packages\django\db\models\query.py", line 101, in __repr__ return repr(self._get_data()) File "C:\Python25\lib\site-packages\django\db\models\query.py", line 444, in _get_data self._result_cache = list(self.iterator()) File "C:\Python25\lib\site-packages\django\db\models\query.py", line 181, in iterator cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params) File "C:\Python25\lib\site-packages\django\db\backends\util.py", line 12, in execute return self.cursor.execute(sql, params) File "C:\Python25\lib\site-packages\django\db\backends\postgresql\base.py", line 43, in execute return self.cursor.execute(sql, [smart_basestring(p, self.charset) for p in params]) ProgrammingError: ERROR: column example_book.stats__times_read does not exist at character 80 SELECT "example_book"."id","example_book"."title" FROM "example_book" ORDER BY "example_book"."stats__times_read" ASC
comment:12 by , 18 years ago
I just found out exactly the same error happens in this case:
from django.db import models class Book(models.Model): title = models.TextField() class BookStat(models.Model): book = models.OneToOneField(Book) times_read = models.PositiveIntegerField() Book.objects.all().order_by("bookstat__times_read")
I.e. can't sort by a one-to-one field in another model.
- bram
comment:13 by , 18 years ago
The patch breaks on:
{{
queryset.extra(select={'squeek': "......"}).order_by("-squeek")
}}
resulting in the error:
Cannot resolve keyword 'comment_count' into field
comment:14 by , 18 years ago
Patch needs improvement: | set |
---|
comment:15 by , 18 years ago
Can you please post your models and the full traceback? How does 'comment_count' get into it?
comment:17 by , 18 years ago
Bram, can you please also send in the full sql statement that was sent to the database? queryset._get_sql_clause()
can do this trick if it isn't visible elsewhere.
comment:18 by , 18 years ago
Sorry, comment_count should have been "squeek".
That said, I had to un-apply the patch and work with the extra( ) to get it all working without the patch as I'm on a very tight deadline. However here's what I found the last time:
Models:
from django.db import models class Book(models.Model): title = models.TextField() class BookStat(models.Model): book = models.OneToOneField(Book) times_read = models.PositiveIntegerField() class BookRating(models.Model): book = models.ForeignKey(Book) rating = models.PositiveIntegerField()
Shell:
from example.models import * bfg = Book.objects.create(title="bfg") bfg.save() matilda = Book.objects.create(title="matilda") matilda.save() BookRating.objects.create(book=bfg, rating=5).save() BookRating.objects.create(book=bfg, rating=8).save() BookRating.objects.create(book=bfg, rating=15).save() BookRating.objects.create(book=matilda, rating=15).save() BookRating.objects.create(book=matilda, rating=16).save() BookRating.objects.create(book=matilda, rating=17).save() BookStat.objects.create(book=bfg, times_read=5).save() BookStat.objects.create(book=matilda, times_read=3).save() # works with patch: qs = Book.objects.all().order_by("stats__times_read") # the equivalent, works without patch, same result qs = qs.extra(where=["example_book.id = example_bookstat.book_id"], tables=["example_bookstat"]).order_by("-example_bookstat.times_read") # works WITHOUT patch, breaks with patch qs = Book.objects.all().extra(select={ 'avg_rating': 'SELECT avg(rating) FROM example_bookrating WHERE example_bookrating.book_id = example_book.id'}).order_by("-avg_rating")
Right now I can't do the stack-trace as I would have to re-apply the patch...
For people looking into these problems, this is a workaround for the initial problem described in the ticket:
comment:19 by , 18 years ago
Cc: | added |
---|
comment:20 by , 18 years ago
Has patch: | unset |
---|---|
Needs tests: | unset |
Patch needs improvement: | unset |
Triage Stage: | Accepted → Unreviewed |
OUCH
The first two patches and the later comments (beginning with comment 9) actually belong to #2076.
See thread in django-developers
Thank you for digging this up, Ramiro Gonzales.
comment:21 by , 18 years ago
So, it seems the patch for #2076 also solves this ticket :-/
Attached you will find a models1.py
, it has some doctest [1]testcases that reproduce the problem described by the original reporter. It should be installed as the models.py
file of a ticket2210
application.
Also attached is a models2.py
, it has the same [1]testcases but using the order_by
notation introduced by the patch for #2076 (so that patch should be applied first) and they complete without errors. It should be installed as the app models.py
file.
Apologies for the noise.
- Run them executing
$ ./manage.py test ticket2210
by , 18 years ago
Attachment: | models1.py added |
---|
by , 18 years ago
Attachment: | models2.py added |
---|
comment:22 by , 18 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:23 by , 18 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Closing this as a duplicate of #2076, since that ticket has a fairly good patch and test cases.
A similar report was made in #2076, also #2121 may be related too?