Django

Code

Ticket #1576 (closed: fixed)

Opened 3 years ago

Last modified 1 year ago

[newforms-admin] Incorrect SQL generated for ordering by related models.

Reported by: k.kosciuszkiewicz at gmail dot com Assigned to: adrian
Milestone: Component: django.contrib.admin
Version: SVN Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

Test case:

class One(meta.Model):
    field_1 = meta.IntegerField()

    class META:
        ordering = ['-field_1']

class Two(meta.Model):
    field_1 = meta.IntegerField()
    fkey    = meta.ForeignKey(One)

    class META:
        admin = meta.Admin()
        ordering = ['fkey']

Displaying list of "Twos" in admin interface results in exception because of invalid SQL is generated with ORDER BY "test_ones"."-field_1" ASC.

Attachments

admin_fk_ordering.patch (1.5 kB) - added by SmileyChris on 04/11/07 00:38:24.

Change History

07/19/06 03:45:13 changed by mtredinnick

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

This has been fixed at some point in the interim.

04/10/07 20:18:05 changed by Wesley Fok <portland@chrominance.net>

  • status changed from closed to reopened.
  • version set to 0.96.
  • resolution deleted.

I've discovered the same problem with 0.96:

## structure/models.py
class Issue(models.Model):
    (...)
    pub_date = models.DateField('Publication date', unique=True)
    (...)

    class Meta:
        ordering            = ('-pub_date',)


## stories/models.py
class Story(models.Model):
    (...)
    issue = models.ForeignKey(Issue)
    (...)

    class Admin:
        ordering        = ('issue',)

Attempting to view a list of stories in the Admin interface returns this:

Traceback (most recent call last):
File "/usr/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  77. response = callback(request, *callback_args, **callback_kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/decorators.py" in _checklogin
  55. return view_func(request, *args, **kwargs)
File "/usr/lib/python2.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  39. response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in change_list
  760. cl = ChangeList(request, model)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in __init__
  572. self.get_results(request)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/views/main.py" in get_results
  630. result_list = list(self.query_set)
File "/usr/lib/python2.4/site-packages/django/db/models/query.py" in __iter__
  108. return iter(self._get_data())
File "/usr/lib/python2.4/site-packages/django/db/models/query.py" in _get_data
  470. self._result_cache = list(self.iterator())
File "/usr/lib/python2.4/site-packages/django/db/models/query.py" in iterator
  183. cursor.execute("SELECT " + (self._distinct and "DISTINCT " or "") + ",".join(select) + sql, params)
File "/usr/lib/python2.4/site-packages/django/db/backends/util.py" in execute
  12. return self.cursor.execute(sql, params)
File "/usr/lib/python2.4/site-packages/django/db/backends/mysql_old/base.py" in execute
  42. return self.cursor.execute(sql, params)
File "/usr/lib/python2.4/site-packages/MySQLdb/cursors.py" in execute
  137. self.errorhandler(self, exc, value)
File "/usr/lib/python2.4/site-packages/MySQLdb/connections.py" in defaulterrorhandler
  33. raise errorclass, errorvalue

  OperationalError at /admin/stories/story/
  (1054, "Unknown column 'structure_issue.-pub_date' in 'order clause'")

The SQL query:

SELECT `stories_story`.`id`,`stories_story`.`head`,`stories_story`.`deck`,`stories_story`.`slug`,`stories_story`.`section_id`,`stories_story`.`issue_id`,`stories_story`.`content`,`stories_story`.`summary`,`stories_story`.`section_order`,`structure_section`.`id`,`structure_section`.`name`,`structure_section`.`short_name`,`structure_section`.`slug`,`structure_issue`.`id`,`structure_issue`.`volume_id`,`structure_issue`.`issue`,`structure_issue`.`extra`,`structure_issue`.`pub_date`,`structure_issue`.`sections_id`,`structure_issue`.`is_published`,`structure_volume`.`id`,`structure_volume`.`volume`,`structure_flatplanconfig`.`id`,`structure_flatplanconfig`.`name` FROM `stories_story` , `structure_section`, `structure_issue`, `structure_volume`, `structure_flatplanconfig` WHERE `stories_story`.`section_id` = `structure_section`.`id` AND `stories_story`.`issue_id` = `structure_issue`.`id` AND `structure_issue`.`volume_id` = `structure_volume`.`id` AND `structure_issue`.`sections_id` = `structure_flatplanconfig`.`id` ORDER BY `structure_issue`.`-pub_date` ASC

Moving the "ordering = ('issue',)" field to the Meta class in Story doesn't help. What does help is if I simply swap the minuses; giving Issues an ordering of 'pub_date' and Stories an ordering of '-issue' works just fine.

04/10/07 23:25:37 changed by SmileyChris

  • stage changed from Unreviewed to Accepted.

Just tested, and I can confirm the above error.

04/10/07 23:25:50 changed by SmileyChris

  • version changed from 0.96 to SVN.

(it's for SVN too)

04/11/07 00:38:24 changed by SmileyChris

  • attachment admin_fk_ordering.patch added.

04/11/07 00:38:58 changed by SmileyChris

  • has_patch set to 1.
  • stage changed from Accepted to Ready for checkin.

Fixed!

04/26/07 08:48:31 changed by mtredinnick

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

(In [5092]) Fixed #1576 -- Fixed incorrect SQL generated when using descending ordering from related models. Patch from SmileyChris?.

04/26/07 09:58:39 changed by mtredinnick

(In [5097]) Revert [5092], since this should only have been applied to newforms-admin. Refs #1576.

04/26/07 09:59:24 changed by mtredinnick

  • status changed from closed to reopened.
  • resolution deleted.
  • summary changed from Incorrect SQL generated for ordering by related models. to [newforms-admin] Incorrect SQL generated for ordering by related models..

04/28/07 10:09:41 changed by mtredinnick

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

(In [5122]) newforms-admin: Fixed #1576 -- Fixed SQL generation when using descending ordering from related models. Based on a patch from SmileyChris?.

(follow-ups: ↓ 11 ↓ 13 ) 08/07/07 16:45:20 changed by gwilson

Malcolm, what was the reason this was applied to newforms-admin instead of trunk? I ask as there are a couple open tickets about this same issue in trunk (#3825 and #5057) that could be marked duplicates of this ticket.

(in reply to: ↑ 10 ) 08/07/07 17:00:47 changed by gwilson

Replying to gwilson:

I ask as there are a couple open tickets about this same issue in trunk (#3825 and #5057) that could be marked duplicates of this ticket.

#2895 could be marked a duplicate of this too if we want to re-open this one. In any case, I have marked #3825 and #5057 as duplicates of #2895.

08/09/07 08:50:22 changed by Ramiro Morales

Just wanted to point to the comment/patch I posted here on #3002

So, after #2076 is solved, or even better after Malcolm's ongoing queryset refactoring efforts lands in trunk and solves these recursive order_by+FK+Meta.ordering problems (I'm hoping it's one of the itemos in his TODO list) maybe this problem can be solved by applying DRY principle: Letting the ORM handle this stuff and removing similar code from the admin app (be it classical or newforms-admin)

(in reply to: ↑ 10 ) 08/20/07 02:31:32 changed by mtredinnick

Replying to gwilson:

Malcolm, what was the reason this was applied to newforms-admin instead of trunk? I ask as there are a couple open tickets about this same issue in trunk (#3825 and #5057) that could be marked duplicates of this ticket.

As per a discussion on django-dev a while back, we're trying not to apply anything we don't absolutely have to against admin in trunk so that merging with newforms-admin is easier and we can focus on getting newforms-admin finished. Sometimes we forget and apply stuff to trunk admin, but as much as possible patches should be rewritten against newforms-admin.


Add/Change #1576 ([newforms-admin] Incorrect SQL generated for ordering by related models.)




Change Properties
Action