From 47ef8f31f30a2b1920f82494d729c285565867dd Mon Sep 17 00:00:00 2001 From: Yohann Gabory Date: Wed, 27 Jul 2016 15:17:05 +0200 Subject: [PATCH] Fixed #13312 -- Allowed specifying the order of null fields in queries. Thanks Mariusz Felisiak for finishing the patch. --- django/db/models/expressions.py | 43 +++++++++++++++++++++------ docs/ref/models/expressions.txt | 16 ++++++++-- docs/releases/1.11.txt | 5 ++++ tests/ordering/models.py | 2 ++ tests/ordering/tests.py | 52 +++++++++++++++++++++++++++++++-- 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() ` 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.