Django

Code

Ticket #3002 (closed: fixed)

Opened 2 years ago

Last modified 2 months ago

admin getting confused by "order_by" statements.

Reported by: terry@weissman.org Assigned to: nobody
Milestone: Component: Admin interface
Version: Keywords: nfa-changelist
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 0

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

ticket-3002.diff (1.2 kB) - added by ramiro <rm0 _at_ gmx.net> on 02/02/07 20:47:41.
A better version of the proposed patch, this time as an attachment
t3002-r7480.diff (1.4 kB) - added by ramiro on 04/27/08 10:32:37.
Update patch to trunk state as of r7480 (post qs-rf merging)
t3002-trunk-r7484.diff (1.4 kB) - added by ramiro on 04/27/08 19:04:19.
Slightly better patch fixing the bug, for trunk as of r7484

Change History

11/07/06 14:13:42 changed by terry@weissman.org

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'")

11/08/06 10:44:42 changed by ubernostrum

  • status changed from new to closed.
  • resolution set to duplicate.

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

02/02/07 15:05:33 changed by ramiro <rm0 _at_ gmx.net>

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.

02/02/07 20:47:41 changed by ramiro <rm0 _at_ gmx.net>

  • attachment ticket-3002.diff added.

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

02/02/07 22:39:05 changed by Gary Wilson <gary.wilson@gmail.com>

  • keywords set to reopen.
  • has_patch set to 1.
  • needs_tests set to 1.
  • stage changed from Unreviewed to Accepted.

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.

02/06/07 10:31:22 changed by jacob

  • status changed from closed to reopened.
  • resolution deleted.

02/06/07 14:42:15 changed by SmileyChris

  • keywords deleted.

10/14/07 17:38:55 changed by mtredinnick

(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).

03/19/08 14:29:11 changed by jakub_vysoky

  • keywords set to nfa-changelist.

04/27/08 10:32:37 changed by ramiro

  • attachment t3002-r7480.diff added.

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

04/27/08 10:43:23 changed by ramiro

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

04/27/08 19:04:19 changed by ramiro

  • attachment t3002-trunk-r7484.diff added.

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

04/27/08 21:40:58 changed by mtredinnick

  • status changed from reopened to closed.
  • resolution set to fixed.

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


Add/Change #3002 (admin getting confused by "order_by" statements.)




Change Properties
Action