Skip to content

Commit 6cf15c9

Browse files
committed
Avoid invalid SQL syntax from OrderBy.as_sql
By making a copy of the OrderBy expression and disabling its nulls_first/nulls_last flags, this prevents Django's core OrderBy.as_sql function from modifying the custom templates SQL Server requires. See <https://code.djangoproject.com/ticket/32584> for details on why this isn't implemented as a feature flag. Fixes #31
1 parent 33b4e23 commit 6cf15c9

File tree

2 files changed

+35
-9
lines changed

2 files changed

+35
-9
lines changed

mssql/functions.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,24 +71,27 @@ def sqlserver_lookup(self, compiler, connection):
7171

7272

7373
def sqlserver_orderby(self, compiler, connection):
74-
# MSSQL doesn't allow ORDER BY EXISTS() unless it's wrapped in
75-
# a CASE WHEN.
76-
7774
template = None
7875
if self.nulls_last:
7976
template = 'CASE WHEN %(expression)s IS NULL THEN 1 ELSE 0 END, %(expression)s %(ordering)s'
8077
if self.nulls_first:
8178
template = 'CASE WHEN %(expression)s IS NULL THEN 0 ELSE 1 END, %(expression)s %(ordering)s'
8279

80+
copy = self.copy()
81+
82+
# Prevent OrderBy.as_sql() from modifying supplied templates
83+
copy.nulls_first = False
84+
copy.nulls_last = False
85+
86+
# MSSQL doesn't allow ORDER BY EXISTS() unless it's wrapped in a CASE WHEN.
8387
if isinstance(self.expression, Exists):
84-
copy = self.copy()
8588
copy.expression = Case(
8689
When(self.expression, then=True),
8790
default=False,
8891
output_field=BooleanField(),
8992
)
90-
return copy.as_sql(compiler, connection, template=template)
91-
return self.as_sql(compiler, connection, template=template)
93+
94+
return copy.as_sql(compiler, connection, template=template)
9295

9396

9497
def split_parameter_list_as_sql(self, compiler, connection):

testapp/tests/test_expressions.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@
44
from unittest import skipUnless
55

66
from django import VERSION
7-
from django.db.models import IntegerField
7+
from django.db.models import IntegerField, F
88
from django.db.models.expressions import Case, Exists, OuterRef, Subquery, Value, When
9-
from django.test import TestCase
9+
from django.test import TestCase, skipUnlessDBFeature
1010

11-
from ..models import Author, Comment, Post
11+
12+
from ..models import Author, Comment, Post, Editor
1213

1314
DJANGO3 = VERSION[0] >= 3
1415

@@ -54,3 +55,25 @@ def test_order_by_exists(self):
5455

5556
authors_by_posts = Author.objects.order_by(Exists(Post.objects.filter(author=OuterRef('pk'))).asc())
5657
self.assertSequenceEqual(authors_by_posts, [author_without_posts, self.author])
58+
59+
60+
@skipUnless(DJANGO3, "Django 3 specific tests")
61+
@skipUnlessDBFeature("order_by_nulls_first")
62+
class TestOrderBy(TestCase):
63+
def setUp(self):
64+
self.author = Author.objects.create(name="author")
65+
self.post = Post.objects.create(title="foo", author=self.author)
66+
self.editor = Editor.objects.create(name="editor")
67+
self.post_alt = Post.objects.create(title="Post with editor", author=self.author, alt_editor=self.editor)
68+
69+
def test_order_by_nulls_last(self):
70+
results = Post.objects.order_by(F("alt_editor").asc(nulls_last=True)).all()
71+
self.assertEqual(len(results), 2)
72+
self.assertIsNotNone(results[0].alt_editor)
73+
self.assertIsNone(results[1].alt_editor)
74+
75+
def test_order_by_nulls_first(self):
76+
results = Post.objects.order_by(F("alt_editor").desc(nulls_first=True)).all()
77+
self.assertEqual(len(results), 2)
78+
self.assertIsNone(results[0].alt_editor)
79+
self.assertIsNotNone(results[1].alt_editor)

0 commit comments

Comments
 (0)