Opened 17 years ago

Closed 16 years ago

#3002 closed defect (fixed)

admin getting confused by "order_by" statements.

Reported by: terry@… Owned by: nobody
Component: contrib.admin Version:
Severity: normal Keywords: nfa-changelist
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here are two simple models. When I try to bring up the admin UI for the "Faq" model, it crashes. The crash seems to show that it is somehow incorrectly getting the "order_by" entry from the FaqCategory class.

class FaqCategory(models.Model):
    name = models.CharField(maxlength=255)
    sortkey = models.IntegerField(default=0)
    def __str__(self):
        return "[Sortkey %d] %s" % (self.sortkey, self.name)

    class Admin:
        pass

    class Meta:
        ordering = ['sortkey', 'name']


class Faq(models.Model):
    category = models.ForeignKey(FaqCategory)
    sortkey = models.IntegerField(default=0)
    question = models.TextField()
    answer = models.TextField()
    def __str__(self):
        return "[Sortkey %d] %s" % (self.sortkey, self.question)

    class Admin:
        pass

    class Meta:
        ordering = ['category', 'sortkey', 'question']
        unique_together = (('category', 'sortkey', 'question'),)

This bug seems very similar to the one described in ticket 1547, but that one claims to be fixed.

Attachments (3)

ticket-3002.diff (1.2 KB ) - added by ramiro <rm0 _at_ gmx.net> 17 years ago.
A better version of the proposed patch, this time as an attachment
t3002-r7480.diff (1.4 KB ) - added by Ramiro Morales 16 years ago.
Update patch to trunk state as of r7480 (post qs-rf merging)
t3002-trunk-r7484.diff (1.4 KB ) - added by Ramiro Morales 16 years ago.
Slightly better patch fixing the bug, for trunk as of r7484

Download all attachments as: .zip

Change History (13)

comment:1 by terry@…, 17 years ago

The offending SQL that is generated is:

SELECT `order_faq`.`id`,`order_faq`.`category_id`,`order_faq`.`sortkey`,`order_faq`.`question`,`order_faq`.`answer`
FROM `order_faq` ORDER BY `order_faqcategory`.`sortkey` ASC

And here's the traceback that the admin generates (when I go to http://127.0.0.1:8000/admin/order/faq/ ):

Traceback (most recent call last):
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/core/handlers/base.py" in get_response
  74. response = callback(request, *callback_args, **callback_kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/contrib/admin/views/main.py" in change_list
  736. cl = ChangeList(request, model)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/contrib/admin/views/main.py" in __init__
  567. self.get_results(request)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/contrib/admin/views/main.py" in get_results
  625. result_list = list(self.query_set)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/db/models/query.py" in __iter__
  103. return iter(self._get_data())
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/db/models/query.py" in _get_data
  430. self._result_cache = list(self.iterator())
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/db/models/query.py" in iterator
  172. cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/db/backends/util.py" in execute
  12. return self.cursor.execute(sql, params)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/Django-0.95-py2.5.egg/django/db/backends/mysql/base.py" in execute
  35. return self.cursor.execute(sql, params)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/MySQLdb/cursors.py" in execute
  163. self.errorhandler(self, exc, value)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/MySQLdb/connections.py" in defaulterrorhandler
  35. raise errorclass, errorvalue

  OperationalError at /admin/order/faq/
  (1054, "Unknown column 'order_faqcategory.sortkey' in 'order clause'")

comment:2 by James Bennett, 17 years ago

Resolution: duplicate
Status: newclosed

I'm pretty sure this is the same as #2076.

comment:3 by ramiro <rm0 _at_ gmx.net>, 17 years ago

It seems to me this ticket is about a different issue that the one described in #2076, that tickets is about a bug in the Django ORM and this one is related to a bug of the Admin contrib app.

Even when a fix for #2076 is applied (by using the patch attached to that ticket), the Admin app keeps generating the bug described here. So I would suggest to reopen this.

This minimal example demonstrates this bug:

from django.db import models

class Poll(models.Model):
    question = models.CharField(maxlength=200)

    def __str__(self):
        return self.question

    class Admin:
        pass

class Choice(models.Model):
    poll = models.ForeignKey(Poll, null=True)
    choice = models.CharField(maxlength=200)

    def __str__(self):
        return self.choice

    class Meta:
        ordering = ('poll',)

    class Admin:
        pass

If you then go to the Admin app index page and click in the Choice link you get the traceback.

If the patch attached to #2076 gets applied then this ticket may be solved by making the Admin app stop handling the order_by+foreing key stuff by itself and leaving the ORM to take care of it. Something like this small patch:

--- a/django/contrib/admin/views/main.py.orig     2006-12-30 12:06:18.000000000 -0300
+++ b/django/contrib/admin/views/main.py  2007-02-02 18:00:39.000000000 -0300
@@ -697,17 +697,6 @@
         # If the order-by field is a field with a relationship, order by the
         # value in the related table.
         lookup_order_field = self.order_field
-        try:
-            f = self.lookup_opts.get_field(self.order_field, many_to_many=False)
-        except models.FieldDoesNotExist:
-            pass
-        else:
-            if isinstance(f.rel, models.OneToOneRel):
-                # For OneToOneFields, don't try to order by the related object's ordering criteria.
-                pass
-            elif isinstance(f.rel, models.ManyToOneRel):
-                rel_ordering = f.rel.to._meta.ordering and f.rel.to._meta.ordering[0] or f.rel.to._meta.pk.column
-                lookup_order_field = '%s.%s' % (f.rel.to._meta.db_table, rel_ordering)

         # Set ordering.
         qs = qs.order_by((self.order_type == 'desc' and '-' or '') + lookup_order_field)

Also, ticket #2895 could then also be possibly closed.

by ramiro <rm0 _at_ gmx.net>, 17 years ago

Attachment: ticket-3002.diff added

A better version of the proposed patch, this time as an attachment

comment:4 by Gary Wilson <gary.wilson@…>, 17 years ago

Has patch: set
Keywords: reopen added
Needs tests: set
Triage Stage: UnreviewedAccepted

This was (wrongly?) marked as a duplicate of #2076, which was (wrongly?) marked a duplicate of #2210.

Also, the original description mentions that this bug seems similar to #1547, which was marked as a duplicate of #1245, which was fixed.

We indeed need some tests.

comment:5 by Jacob, 17 years ago

Resolution: duplicate
Status: closedreopened

comment:6 by Chris Beaven, 17 years ago

Keywords: reopen removed

comment:7 by Malcolm Tredinnick, 16 years ago

(In [6510]) queryset-refactor: Fixed a large bag of order_by() problems.

This also picked up a small bug in some twisted select_related() handling.

Introduces a new syntax for cross-model ordering: foobarbaz, using field
names, instead of a strange combination of table names and field names. This
might turn out to be backwards compatible (although the old syntax leads to
bugs and is not to be recommended).

Still to come: fixes for extra() handling, since the new syntax can't handle
that and doc updates.

Things are starting to get a bit slow here, so we might eventually have to
remove ordering by many-many and other multiple-result fields, since they don't
make a lot of sense in any case. For now, it's legal.

Refs #2076, #2874, #3002 (although the admin bit doesn't work yet).

comment:8 by jakub_vysoky, 16 years ago

Keywords: nfa-changelist added

by Ramiro Morales, 16 years ago

Attachment: t3002-r7480.diff added

Update patch to trunk state as of r7480 (post qs-rf merging)

comment:9 by Ramiro Morales, 16 years ago

Updated patch so it applies cleanly to trunk, now that qs-rf landed it is needed even more because of deprecated ordering syntax usage. Don't know if this brokenness will warrant core devs fixing this because of the no-bug-fixing-efforts policy regarding the admin app in trunk, posting it anyways because it may be of help to people finding FieldError tracebacks (that's the qs-rf introduced exception being thrown now) tracebacks.

Remembered about this ticket when looking at #7094

by Ramiro Morales, 16 years ago

Attachment: t3002-trunk-r7484.diff added

Slightly better patch fixing the bug, for trunk as of r7484

comment:10 by Malcolm Tredinnick, 16 years ago

Resolution: fixed
Status: reopenedclosed

(In [7491]) Fixed #3002 -- Fixed a problem with ordering by related models in the admin
interface. Patch from Ramiro Morales.

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