Fixed #13312 -- Allowed specifying the order of null fields in queries.

Thanks Mariusz Felisiak for finishing the patch.

5 files changed, 105 insertions, 13 deletions

diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py
index 6bfbc36534..5c137edfc3 100644
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -319,11 +319,11 @@ class BaseExpression(object):
         """
         return [e._output_field_or_none for e in self.get_source_expressions()]
 
-    def asc(self):
-        return OrderBy(self)
+    def asc(self, **kwargs):
+        return OrderBy(self, **kwargs)
 
-    def desc(self):
-        return OrderBy(self, descending=True)
+    def desc(self, **kwargs):
+        return OrderBy(self, descending=True, **kwargs)
 
     def reverse_ordering(self):
         return self
@@ -462,11 +462,11 @@ class F(Combinable):
     def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False):
         return query.resolve_ref(self.name, allow_joins, reuse, summarize)
 
-    def asc(self):
-        return OrderBy(self)
+    def asc(self, **kwargs):
+        return OrderBy(self, **kwargs)
 
-    def desc(self):
-        return OrderBy(self, descending=True)
+    def desc(self, **kwargs):
+        return OrderBy(self, descending=True, **kwargs)
 
 
 class Func(Expression):
@@ -869,7 +869,11 @@ class Case(Expression):
 class OrderBy(BaseExpression):
     template = '%(expression)s %(ordering)s'
 
-    def __init__(self, expression, descending=False):
+    def __init__(self, expression, descending=False, nulls_first=False, nulls_last=False):
+        if nulls_first and nulls_last:
+            raise ValueError('nulls_first and nulls_last are mutually exclusive')
+        self.nulls_first = nulls_first
+        self.nulls_last = nulls_last
         self.descending = descending
         if not hasattr(expression, 'resolve_expression'):
             raise ValueError('expression must be an expression type')
@@ -886,6 +890,11 @@ class OrderBy(BaseExpression):
         return [self.expression]
 
     def as_sql(self, compiler, connection, template=None, **extra_context):
+        if not template:
+            if self.nulls_last:
+                template = '%s NULLS LAST' % self.template
+            elif self.nulls_first:
+                template = '%s NULLS FIRST' % self.template
         connection.ops.check_expression_support(self)
         expression_sql, params = compiler.compile(self.expression)
         placeholders = {
@@ -896,6 +905,22 @@ class OrderBy(BaseExpression):
         template = template or self.template
         return (template % placeholders).rstrip(), params
 
+    def as_sqlite(self, compiler, connection):
+        template = None
+        if self.nulls_last:
+            template = '%(expression)s IS NULL, %(expression)s %(ordering)s'
+        elif self.nulls_first:
+            template = '%(expression)s IS NOT NULL, %(expression)s %(ordering)s'
+        return self.as_sql(compiler, connection, template=template)
+
+    def as_mysql(self, compiler, connection):
+        template = None
+        if self.nulls_last:
+            template = 'IF(ISNULL(%(expression)s),1,0), %(expression)s %(ordering)s '
+        elif self.nulls_first:
+            template = 'IF(ISNULL(%(expression)s),0,1), %(expression)s %(ordering)s '
+        return self.as_sql(compiler, connection, template=template)
+
     def get_group_by_cols(self):
         cols = []
         for source in self.get_source_expressions():
diff --git a/docs/ref/models/expressions.txt b/docs/ref/models/expressions.txt
index 91786b3622..e46df22f98 100644
--- a/docs/ref/models/expressions.txt
+++ b/docs/ref/models/expressions.txt
@@ -560,14 +560,26 @@ calling the appropriate methods on the wrapped expression.
         nested expressions. ``F()`` objects, in particular, hold a reference
         to a column.
 
-    .. method:: asc()
+    .. method:: asc(nulls_first=False, nulls_last=False)
 
         Returns the expression ready to be sorted in ascending order.
 
-    .. method:: desc()
+        ``nulls_first`` and ``nulls_last`` define how null values are sorted.
+
+        .. versionchanged:: 1.11
+
+            The ``nulls_last`` and ``nulls_first`` parameters were added.
+
+    .. method:: desc(nulls_first=False, nulls_last=False)
 
         Returns the expression ready to be sorted in descending order.
 
+        ``nulls_first`` and ``nulls_last`` define how null values are sorted.
+
+        .. versionchanged:: 1.11
+
+            The ``nulls_first`` and ``nulls_last`` parameters were added.
+
     .. method:: reverse_ordering()
 
         Returns ``self`` with any modifications required to reverse the sort
diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt
index 59b7fb54f9..2035588af6 100644
--- a/docs/releases/1.11.txt
+++ b/docs/releases/1.11.txt
@@ -337,6 +337,11 @@ Models
 * You can now use the ``unique=True`` option with
   :class:`~django.db.models.FileField`.
 
+* Added the ``nulls_first`` and ``nulls_last`` parameters to
+  :class:`Expression.asc() <django.db.models.Expression.asc>` and
+  :meth:`~django.db.models.Expression.desc` to control
+  the ordering of null values.
+
 Requests and Responses
 ~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/tests/ordering/models.py b/tests/ordering/models.py
index 2c4e9b3308..1f794ca3a1 100644
--- a/tests/ordering/models.py
+++ b/tests/ordering/models.py
@@ -18,6 +18,8 @@ from django.utils.encoding import python_2_unicode_compatible
 
 
 class Author(models.Model):
+    name = models.CharField(max_length=63, null=True, blank=True)
+
     class Meta:
         ordering = ('-pk',)
 
diff --git a/tests/ordering/tests.py b/tests/ordering/tests.py
index f19cbdeddc..399b8d7035 100644
--- a/tests/ordering/tests.py
+++ b/tests/ordering/tests.py
@@ -4,6 +4,7 @@ from datetime import datetime
 from operator import attrgetter
 
 from django.db.models import F
+from django.db.models.functions import Upper
 from django.test import TestCase
 
 from .models import Article, Author, Reference
@@ -17,8 +18,8 @@ class OrderingTests(TestCase):
         cls.a2 = Article.objects.create(headline="Article 2", pub_date=datetime(2005, 7, 27))
         cls.a3 = Article.objects.create(headline="Article 3", pub_date=datetime(2005, 7, 27))
         cls.a4 = Article.objects.create(headline="Article 4", pub_date=datetime(2005, 7, 28))
-        cls.author_1 = Author.objects.create()
-        cls.author_2 = Author.objects.create()
+        cls.author_1 = Author.objects.create(name="Name 1")
+        cls.author_2 = Author.objects.create(name="Name 2")
         for i in range(2):
             Author.objects.create()
 
@@ -88,6 +89,53 @@ class OrderingTests(TestCase):
             attrgetter("headline")
         )
 
+    def test_order_by_nulls_first_and_last(self):
+        msg = "nulls_first and nulls_last are mutually exclusive"
+        with self.assertRaisesMessage(ValueError, msg):
+            Article.objects.order_by(F("author").desc(nulls_last=True, nulls_first=True))
+
+    def test_order_by_nulls_last(self):
+        Article.objects.filter(headline="Article 3").update(author=self.author_1)
+        Article.objects.filter(headline="Article 4").update(author=self.author_2)
+        # asc and desc are chainable with nulls_last.
+        self.assertSequenceEqual(
+            Article.objects.order_by(F("author").desc(nulls_last=True)),
+            [self.a4, self.a3, self.a1, self.a2],
+        )
+        self.assertSequenceEqual(
+            Article.objects.order_by(F("author").asc(nulls_last=True)),
+            [self.a3, self.a4, self.a1, self.a2],
+        )
+        self.assertSequenceEqual(
+            Article.objects.order_by(Upper("author__name").desc(nulls_last=True)),
+            [self.a4, self.a3, self.a1, self.a2],
+        )
+        self.assertSequenceEqual(
+            Article.objects.order_by(Upper("author__name").asc(nulls_last=True)),
+            [self.a3, self.a4, self.a1, self.a2],
+        )
+
+    def test_order_by_nulls_first(self):
+        Article.objects.filter(headline="Article 3").update(author=self.author_1)
+        Article.objects.filter(headline="Article 4").update(author=self.author_2)
+        # asc and desc are chainable with nulls_first.
+        self.assertSequenceEqual(
+            Article.objects.order_by(F("author").asc(nulls_first=True)),
+            [self.a1, self.a2, self.a3, self.a4],
+        )
+        self.assertSequenceEqual(
+            Article.objects.order_by(F("author").desc(nulls_first=True)),
+            [self.a1, self.a2, self.a4, self.a3],
+        )
+        self.assertSequenceEqual(
+            Article.objects.order_by(Upper("author__name").asc(nulls_first=True)),
+            [self.a1, self.a2, self.a3, self.a4],
+        )
+        self.assertSequenceEqual(
+            Article.objects.order_by(Upper("author__name").desc(nulls_first=True)),
+            [self.a1, self.a2, self.a4, self.a3],
+        )
+
     def test_stop_slicing(self):
         """
         Use the 'stop' part of slicing notation to limit the results.