Opened 17 years ago

Closed 15 years ago

#2884 closed defect (fixed)

[newforms-admin] Incorrect sql generated when ordering a queryset where the related model's Meta.ordering has an FK

Reported by: bangcok_dangerus ( at ] hotmail.com Owned by: nobody
Component: contrib.admin Version: dev
Severity: normal Keywords:
Cc: real.human@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Description: In the admin, clicking to sort on a field specified in list_display causes a ProgrammingError due to a non-existent column in the SQL ORDER BY clause.

How to replicate: This happens when the field specified in list_display is a ForeignKey to another object, and this related object contains a Meta ordering option which is also a ForeignKey. Example minimal models.py:

from django.db import models

class Subject(models.Model):
	name = models.CharField(maxlength=40)
	def __str__(self):
		return self.name
	class Admin:
		pass

class Course(models.Model):
	name = models.CharField(maxlength=40)
	subject = models.ForeignKey(Subject)
	def __str__(self):
		return self.name
	class Admin:
		list_display = ('__str__', 'subject')
	class Meta:
		ordering = ["subject"]

class Class(models.Model):
	course = models.ForeignKey(Course)
	name = models.CharField(maxlength = 40)
	def __str__(self):
		return self.name
	class Admin:
		list_display = ('__str__','course')
	class Meta:
		verbose_name_plural = "Classes"

Navigating to the Classes page and clicking on the 'course' column to order will generate the error.

Cause: This is a bug in django/contrib/admin/views/main.py . The code in ChangeList.get_query_set wrongly assumes that the ordering option in the related object is a field in the related object's table. For the above model, the incorrect ORDER BY clause currently generated is 'test_course.subject'.

Fix: This patch recursively follows any ForeignKeys in the ordering options to find the correct ORDER BY clause, in this case 'test_subject.name'. In the case of circular ForeignKeys with circular ordering, it will order by the related object's raw ForeignKey field (e.g. course.subject_id)

-Basil

Attachments (2)

get_query_set.patch (2.1 KB ) - added by bangcok_dangerus ( at ] hotmail.com 17 years ago.
fixes the issue, could do with more testing
t2884.py (652 bytes ) - added by Simon G. <dev@…> 17 years ago.

Download all attachments as: .zip

Change History (8)

by bangcok_dangerus ( at ] hotmail.com, 17 years ago

Attachment: get_query_set.patch added

fixes the issue, could do with more testing

comment:1 by Tai Lee, 17 years ago

Component: Admin interfaceDatabase wrapper

this is not only an admin issue. it happens when ordering any queryset (in the admin or any 3rd party code) by a foreign key field, where the related model's default Meta.ordering also includes a foreign key.

comment:2 by mrmachine <real dot human at mrmachine dot net>, 17 years ago

Cc: real.human@… added

comment:3 by Simon G. <dev@…>, 17 years ago

Component: Database wrapperAdmin interface
Patch needs improvement: set
Summary: [patch] incorrect sql generated for sort in adminincorrect sql generated when ordering a queryset where the related model's Meta.ordering has an FK
Triage Stage: UnreviewedReady for checkin

I can replicate this in the admin framework, but not outside the admin. I've attached the test I was using in case anyone can improve them.

by Simon G. <dev@…>, 17 years ago

Attachment: t2884.py added

comment:4 by Malcolm Tredinnick, 17 years ago

Summary: incorrect sql generated when ordering a queryset where the related model's Meta.ordering has an FK[newforms-admin] Incorrect sql generated when ordering a queryset where the related model's Meta.ordering has an FK

The patch is more or less correct for newforms-admin, although I doubt it will apply automatically because the line numbers have changed. Not totally in love with the idea of another nested function, but that can be fixed when committing.

comment:5 by Jacob, 17 years ago

Triage Stage: Ready for checkinAccepted

This should be deferred until after queryset-refactor.

comment:6 by Malcolm Tredinnick, 15 years ago

Resolution: fixed
Status: newclosed

Since ordering by fields that are relations Just Works now, this shouldn't be a problem any longer.

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