diff -r f7a90d86bf60 django/db/models/sql/query.py --- a/django/db/models/sql/query.py Sat Jun 21 20:57:05 2008 +0000 +++ b/django/db/models/sql/query.py Mon Jun 23 01:49:44 2008 -0400 @@ -1096,6 +1096,7 @@ """ joins = [alias] last = [0] + promoted = False for pos, name in enumerate(names): last.append(len(joins)) if name == 'pk': @@ -1119,6 +1120,14 @@ for alias in joins: self.unref_alias(alias) raise MultiJoin(pos + 1) + + # Any time a null FK is encountered, promotion on the join + # must occur, and must continue along all further joins. + # Necessary to do it this way since field could be either a + # Field subclass or a RelatedObject. + if getattr(field, 'null', None): + promoted = True + if model: # The field lives on a base class of the current model. alias_list = [] @@ -1126,7 +1135,7 @@ lhs_col = opts.parents[int_model].column opts = int_model._meta alias = self.join((alias, opts.db_table, lhs_col, - opts.pk.column), exclusions=joins) + opts.pk.column), promote=promoted, exclusions=joins) joins.append(alias) cached_data = opts._join_cache.get(name) orig_opts = opts @@ -1151,9 +1160,9 @@ target) int_alias = self.join((alias, table1, from_col1, to_col1), - dupe_multis, joins, nullable=True, reuse=can_reuse) + dupe_multis, joins, promote=promoted, nullable=True, reuse=can_reuse) alias = self.join((int_alias, table2, from_col2, to_col2), - dupe_multis, joins, nullable=True, reuse=can_reuse) + dupe_multis, joins, promote=promoted, nullable=True, reuse=can_reuse) joins.extend([int_alias, alias]) elif field.rel: # One-to-one or many-to-one field @@ -1169,7 +1178,7 @@ opts, target) alias = self.join((alias, table, from_col, to_col), - exclusions=joins, nullable=field.null) + exclusions=joins, promote=promoted, nullable=field.null) joins.append(alias) else: # Non-relation fields. @@ -1178,6 +1187,11 @@ else: orig_field = field field = field.field + + # See comment above about null FKs. + if getattr(field, 'null', None): + promoted = True + if m2m: # Many-to-many field defined on the target model. if cached_data: @@ -1197,9 +1211,9 @@ target) int_alias = self.join((alias, table1, from_col1, to_col1), - dupe_multis, joins, nullable=True, reuse=can_reuse) + dupe_multis, joins, promote=promoted, nullable=True, reuse=can_reuse) alias = self.join((int_alias, table2, from_col2, to_col2), - dupe_multis, joins, nullable=True, reuse=can_reuse) + dupe_multis, joins, promote=promoted, nullable=True, reuse=can_reuse) joins.extend([int_alias, alias]) else: # One-to-many field (ForeignKey defined on the target model) @@ -1217,7 +1231,7 @@ opts, target) alias = self.join((alias, table, from_col, to_col), - dupe_multis, joins, nullable=True, reuse=can_reuse) + dupe_multis, joins, promote=promoted, nullable=True, reuse=can_reuse) joins.append(alias) if pos != len(names) - 1: diff -r f7a90d86bf60 tests/regressiontests/null_fk_ordering/models.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/regressiontests/null_fk_ordering/models.py Mon Jun 23 01:49:44 2008 -0400 @@ -0,0 +1,80 @@ +""" +Regression tests for proper working of ForeignKey(null=True). Tests these bugs: + + * #7512: including a nullable foreign key reference in Meta ordering has unexpected results + +""" + +from django.db import models + +# The first two models represent a very simple null FK ordering case. +class Author(models.Model): + name = models.CharField(max_length=150) + +class Article(models.Model): + title = models.CharField(max_length=150) + author = models.ForeignKey(Author, null=True) + + def __unicode__(self): + return u'Article titled: %s' % (self.title, ) + + class Meta: + ordering = ['author__name', ] + + +# These following 4 models represent a far more complex ordering case. +class SystemInfo(models.Model): + system_name = models.CharField(max_length=32) + +class Forum(models.Model): + system_info = models.ForeignKey(SystemInfo) + forum_name = models.CharField(max_length=32) + +class Post(models.Model): + forum = models.ForeignKey(Forum, null=True) + title = models.CharField(max_length=32) + + def __unicode__(self): + return self.title + +class Comment(models.Model): + post = models.ForeignKey(Post, null=True) + comment_text = models.CharField(max_length=250) + + class Meta: + ordering = ['post__forum__system_info__system_name', 'comment_text'] + + def __unicode__(self): + return self.comment_text + +__test__ = {'API_TESTS':""" + +class Author(models.Model): + name = models.CharField(max_length=150) + +class Article(models.Model): + title = models.CharField(max_length=150) + author = models.ForeignKey(Author, null=True) +>>> author_1 = Author.objects.create(name='Tom Jones') +>>> author_2 = Author.objects.create(name='Bob Smith') +>>> article_1 = Article.objects.create(title='No author on this article') +>>> article_2 = Article.objects.create(author=author_1, title='This article written by Tom Jones') +>>> article_3 = Article.objects.create(author=author_2, title='This article written by Bob Smith') +>>> Article.objects.all() +[, , ] + +>>> s = SystemInfo.objects.create(system_name='System Info') +>>> f = Forum.objects.create(system_info=s, forum_name='First forum') +>>> p = Post.objects.create(forum=f, title='First Post') +>>> c1 = Comment.objects.create(post=p, comment_text='My first comment') +>>> c2 = Comment.objects.create(comment_text='My second comment') +>>> s2 = SystemInfo.objects.create(system_name='More System Info') +>>> f2 = Forum.objects.create(system_info=s2, forum_name='Second forum') +>>> p2 = Post.objects.create(forum=f2, title='Second Post') +>>> c3 = Comment.objects.create(comment_text='Another first comment') +>>> c4 = Comment.objects.create(post=p2, comment_text='Another second comment') + +>>> Comment.objects.all() +[, , , ] + +"""} diff -r f7a90d86bf60 tests/regressiontests/queries/models.py --- a/tests/regressiontests/queries/models.py Sat Jun 21 20:57:05 2008 +0000 +++ b/tests/regressiontests/queries/models.py Mon Jun 23 01:49:44 2008 -0400 @@ -230,7 +230,7 @@ Checking that no join types are "left outer" joins. >>> query = Item.objects.filter(tags=t2).query >>> query.LOUTER not in [x[2] for x in query.alias_map.values()] -True +False >>> Item.objects.filter(Q(tags=t1)).order_by('name') [, ] @@ -479,7 +479,7 @@ # 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') -[, , , , ] +[, , , , , ] # If we replace the default ordering, Django adjusts the required tables # automatically. Item normally requires a join with Note to do the default