Skip to content

Commit

Permalink
Fixed #13312 -- Added a way to customize the order_by() of null fields
Browse files Browse the repository at this point in the history
  • Loading branch information
boblefrag committed Aug 18, 2016
1 parent 44a6b40 commit f47ba4d
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 16 deletions.
36 changes: 31 additions & 5 deletions django/db/models/expressions.py
Expand Up @@ -462,11 +462,11 @@ def __repr__(self):
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):
Expand Down Expand Up @@ -862,7 +862,12 @@ def as_sql(self, compiler, connection, template=None, case_joiner=None, **extra_
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 mutualy 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')
Expand All @@ -879,6 +884,11 @@ def get_source_expressions(self):
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 = {
Expand All @@ -889,6 +899,22 @@ def as_sql(self, compiler, connection, template=None, **extra_context):
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():
Expand Down
76 changes: 65 additions & 11 deletions tests/ordering/tests.py
Expand Up @@ -10,19 +10,25 @@


class OrderingTests(TestCase):
def setUp(self):
self.a1 = Article.objects.create(

@classmethod
def setUpTestData(cls):
cls.a1 = Article.objects.create(
headline="Article 1", pub_date=datetime(2005, 7, 26)
)
self.a2 = Article.objects.create(
cls.a2 = Article.objects.create(
headline="Article 2", pub_date=datetime(2005, 7, 27)
)
self.a3 = Article.objects.create(
cls.a3 = Article.objects.create(
headline="Article 3", pub_date=datetime(2005, 7, 27)
)
self.a4 = Article.objects.create(
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()
for i in range(3):
Author.objects.create()

def test_default_ordering(self):
"""
Expand Down Expand Up @@ -90,6 +96,58 @@ def test_order_by_override(self):
attrgetter("headline")
)

def test_order_by_nulls_first_and_last(self):
with self.assertRaisesMessage(ValueError,
"nulls_first and nulls_last are mutualy exclusive"):
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)

# check that asc and desc are chainable with nulls_last

self.assertQuerysetEqual(
Article.objects.order_by(F("author").desc(nulls_last=True)), [
"Article 4",
"Article 3",
"Article 1",
"Article 2",
],
attrgetter("headline")
)

self.assertQuerysetEqual(
Article.objects.order_by(F("author").asc(nulls_last=True)), [
"Article 3",
"Article 4",
"Article 1",
"Article 2",
],
attrgetter("headline")
)

def test_order_by_nulls_first(self):
self.assertQuerysetEqual(
Article.objects.order_by(F("author").asc(nulls_first=True)), [
"Article 1",
"Article 2",
"Article 3",
"Article 4",
],
attrgetter("headline")
)

self.assertQuerysetEqual(
Article.objects.order_by(F("author").desc(nulls_first=True)), [
"Article 1",
"Article 2",
"Article 3",
"Article 4",
],
attrgetter("headline")
)

def test_stop_slicing(self):
"""
Use the 'stop' part of slicing notation to limit the results.
Expand Down Expand Up @@ -213,14 +271,10 @@ def test_order_by_pk(self):
Ensure that 'pk' works as an ordering option in Meta.
Refs #8291.
"""
Author.objects.create(pk=1)
Author.objects.create(pk=2)
Author.objects.create(pk=3)
Author.objects.create(pk=4)

self.assertQuerysetEqual(
Author.objects.all(), [
4, 3, 2, 1
5, 4, 3, 2, 1
],
attrgetter("pk")
)
Expand All @@ -232,7 +286,7 @@ def test_order_by_fk_attname(self):
Refs #19195.
"""
for i in range(1, 5):
author = Author.objects.create(pk=i)
author = Author.objects.get(pk=i)
article = getattr(self, "a%d" % (5 - i))
article.author = author
article.save(update_fields={'author'})
Expand Down

0 comments on commit f47ba4d

Please sign in to comment.