mirror of
https://github.com/django/django.git
synced 2025-05-11 17:36:28 +00:00
[5.1.x] Fixed #35665 -- Fixed a crash when passing an empty order_by to Window.
This also caused un-ordered sliced prefetches to crash as they rely on Window. Regression in e16d0c176e9b89628cdec5e58c418378c4a2436a that made OrderByList piggy-back ExpressionList without porting the empty handling that the latter provided. Supporting explicit empty ordering on Window functions and slicing is arguably a foot-gun design due to how backends will return undeterministic results but this is a problem that requires a larger discussion. Refs #35064. Thanks Andrew Backer for the report and Mariusz for the review. Backport of 602fe961e6834d665f2359087a1272e9f9806b71 from main.
This commit is contained in:
parent
dbca05698a
commit
df236b0bcb
@ -116,6 +116,7 @@ class DatabaseFeatures(BaseDatabaseFeatures):
|
|||||||
"Oracle requires ORDER BY in row_number, ANSI:SQL doesn't.": {
|
"Oracle requires ORDER BY in row_number, ANSI:SQL doesn't.": {
|
||||||
"expressions_window.tests.WindowFunctionTests."
|
"expressions_window.tests.WindowFunctionTests."
|
||||||
"test_row_number_no_ordering",
|
"test_row_number_no_ordering",
|
||||||
|
"prefetch_related.tests.PrefetchLimitTests.test_empty_order",
|
||||||
},
|
},
|
||||||
"Oracle doesn't support changing collations on indexed columns (#33671).": {
|
"Oracle doesn't support changing collations on indexed columns (#33671).": {
|
||||||
"migrations.test_operations.OperationTests."
|
"migrations.test_operations.OperationTests."
|
||||||
|
@ -1377,16 +1377,14 @@ class ExpressionList(Func):
|
|||||||
|
|
||||||
template = "%(expressions)s"
|
template = "%(expressions)s"
|
||||||
|
|
||||||
def __init__(self, *expressions, **extra):
|
|
||||||
if not expressions:
|
|
||||||
raise ValueError(
|
|
||||||
"%s requires at least one expression." % self.__class__.__name__
|
|
||||||
)
|
|
||||||
super().__init__(*expressions, **extra)
|
|
||||||
|
|
||||||
def __str__(self):
|
def __str__(self):
|
||||||
return self.arg_joiner.join(str(arg) for arg in self.source_expressions)
|
return self.arg_joiner.join(str(arg) for arg in self.source_expressions)
|
||||||
|
|
||||||
|
def as_sql(self, *args, **kwargs):
|
||||||
|
if not self.source_expressions:
|
||||||
|
return "", ()
|
||||||
|
return super().as_sql(*args, **kwargs)
|
||||||
|
|
||||||
def as_sqlite(self, compiler, connection, **extra_context):
|
def as_sqlite(self, compiler, connection, **extra_context):
|
||||||
# Casting to numeric is unnecessary.
|
# Casting to numeric is unnecessary.
|
||||||
return self.as_sql(compiler, connection, **extra_context)
|
return self.as_sql(compiler, connection, **extra_context)
|
||||||
|
@ -9,4 +9,6 @@ Django 5.1.1 fixes several bugs in 5.1.
|
|||||||
Bugfixes
|
Bugfixes
|
||||||
========
|
========
|
||||||
|
|
||||||
* ...
|
* Fixed a regression in Django 5.1 that caused a crash of ``Window()`` when
|
||||||
|
passing an empty sequence to the ``order_by`` parameter, and a crash of
|
||||||
|
``Prefetch()`` for a sliced queryset without ordering (:ticket:`35665`).
|
||||||
|
@ -2311,11 +2311,6 @@ class ValueTests(TestCase):
|
|||||||
self.assertNotEqual(value, other_value)
|
self.assertNotEqual(value, other_value)
|
||||||
self.assertNotEqual(value, no_output_field)
|
self.assertNotEqual(value, no_output_field)
|
||||||
|
|
||||||
def test_raise_empty_expressionlist(self):
|
|
||||||
msg = "ExpressionList requires at least one expression"
|
|
||||||
with self.assertRaisesMessage(ValueError, msg):
|
|
||||||
ExpressionList()
|
|
||||||
|
|
||||||
def test_compile_unresolved(self):
|
def test_compile_unresolved(self):
|
||||||
# This test might need to be revisited later on if #25425 is enforced.
|
# This test might need to be revisited later on if #25425 is enforced.
|
||||||
compiler = Time.objects.all().query.get_compiler(connection=connection)
|
compiler = Time.objects.all().query.get_compiler(connection=connection)
|
||||||
|
@ -928,6 +928,20 @@ class WindowFunctionTests(TestCase):
|
|||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_empty_ordering(self):
|
||||||
|
"""
|
||||||
|
Explicit empty ordering makes little sense but it is something that
|
||||||
|
was historically allowed.
|
||||||
|
"""
|
||||||
|
qs = Employee.objects.annotate(
|
||||||
|
sum=Window(
|
||||||
|
expression=Sum("salary"),
|
||||||
|
partition_by="department",
|
||||||
|
order_by=[],
|
||||||
|
)
|
||||||
|
).order_by("department", "sum")
|
||||||
|
self.assertEqual(len(qs), 12)
|
||||||
|
|
||||||
def test_related_ordering_with_count(self):
|
def test_related_ordering_with_count(self):
|
||||||
qs = Employee.objects.annotate(
|
qs = Employee.objects.annotate(
|
||||||
department_sum=Window(
|
department_sum=Window(
|
||||||
|
@ -1999,6 +1999,21 @@ class PrefetchLimitTests(TestDataMixin, TestCase):
|
|||||||
with self.assertRaisesMessage(NotSupportedError, msg):
|
with self.assertRaisesMessage(NotSupportedError, msg):
|
||||||
list(Book.objects.prefetch_related(Prefetch("authors", authors[1:])))
|
list(Book.objects.prefetch_related(Prefetch("authors", authors[1:])))
|
||||||
|
|
||||||
|
@skipUnlessDBFeature("supports_over_clause")
|
||||||
|
def test_empty_order(self):
|
||||||
|
authors = Author.objects.order_by()
|
||||||
|
with self.assertNumQueries(3):
|
||||||
|
books = list(
|
||||||
|
Book.objects.prefetch_related(
|
||||||
|
Prefetch("authors", authors),
|
||||||
|
Prefetch("authors", authors[:1], to_attr="authors_sliced"),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
for book in books:
|
||||||
|
with self.subTest(book=book):
|
||||||
|
self.assertEqual(len(book.authors_sliced), 1)
|
||||||
|
self.assertIn(book.authors_sliced[0], list(book.authors.all()))
|
||||||
|
|
||||||
|
|
||||||
class DeprecationTests(TestCase):
|
class DeprecationTests(TestCase):
|
||||||
def test_get_current_queryset_warning(self):
|
def test_get_current_queryset_warning(self):
|
||||||
|
Loading…
x
Reference in New Issue
Block a user