From ba015e0a79f6e51a2253fb0b95c9a68acb2d6090 Mon Sep 17 00:00:00 2001 From: Malcolm Tredinnick Date: Thu, 26 Jun 2008 06:50:22 +0000 Subject: [PATCH] Fixed #7181 -- when ordering by a potentially NULL field, use a left-outer join so that the ordering doesn't accidentally restrict the result set. (Ironically, one existing test actually showed this problem, but I was too dumb to notice the result was incorrect.) git-svn-id: http://code.djangoproject.com/svn/django/trunk@7761 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/sql/query.py | 4 ++++ tests/regressiontests/queries/models.py | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index b30c6a355c..e8d10bc55b 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -610,6 +610,10 @@ class Query(object): alias = joins[-1] col = target.column + # Must use left outer joins for nullable fields. + for join in joins: + self.promote_alias(join) + # If we get to this point and the field is a relation to another model, # append the default ordering for that model. if field.rel and len(joins) > 1 and opts.ordering: diff --git a/tests/regressiontests/queries/models.py b/tests/regressiontests/queries/models.py index d7c8fba9c6..a3e5561b86 100644 --- a/tests/regressiontests/queries/models.py +++ b/tests/regressiontests/queries/models.py @@ -499,7 +499,7 @@ FieldError: Infinite loop caused by ordering. # Ordering by a many-valued attribute (e.g. a many-to-many or reverse # ForeignKey) is legal, but the results might not make sense. That isn't # Django's problem. Garbage in, garbage out. ->>> Item.objects.all().order_by('tags', 'id') +>>> Item.objects.filter(tags__isnull=False).order_by('tags', 'id') [, , , , ] # If we replace the default ordering, Django adjusts the required tables @@ -762,5 +762,11 @@ Bug #7076 -- excluding shouldn't eliminate NULL entries. >>> Tag.objects.exclude(parent__name=t1.name) [, , ] +Bug #7181 -- ordering by related tables should accomodate nullable fields (this +test is a little tricky, since NULL ordering is database dependent. Instead, we +just count the number of results). +>>> len(Tag.objects.order_by('parent__name')) +5 + """}