Skip to content

Commit 8e3839d

Browse files
committed
Fix orderby crash.
1 parent feb4712 commit 8e3839d

File tree

4 files changed

+61
-129
lines changed

4 files changed

+61
-129
lines changed

django_mongodb/compiler.py

Lines changed: 49 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
import itertools
22

3+
from bson import SON
34
from django.core.exceptions import EmptyResultSet, FullResultSet
45
from django.db import DatabaseError, IntegrityError, NotSupportedError
56
from django.db.models import Count, Expression
67
from django.db.models.aggregates import Aggregate, Variance
7-
from django.db.models.expressions import Col, OrderBy, Value
8+
from django.db.models.expressions import Col, Ref, Value
89
from django.db.models.functions.comparison import Coalesce
910
from django.db.models.functions.math import Power
1011
from django.db.models.sql import compiler
11-
from django.db.models.sql.constants import GET_ITERATOR_CHUNK_SIZE, MULTI, ORDER_DIR, SINGLE
12+
from django.db.models.sql.constants import GET_ITERATOR_CHUNK_SIZE, MULTI, SINGLE
1213
from django.utils.functional import cached_property
14+
from pymongo import ASCENDING, DESCENDING
1315

1416
from .base import Cursor
1517
from .query import MongoQuery, wrap_database_errors
@@ -24,6 +26,7 @@ class SQLCompiler(compiler.SQLCompiler):
2426
def __init__(self, *args, **kwargs):
2527
super().__init__(*args, **kwargs)
2628
self._group_pipeline = None
29+
self._order_by = None
2730

2831
def _get_group_alias_column(self, col, annotation_group_idx):
2932
"""Generate alias and replacement for group columns."""
@@ -71,7 +74,7 @@ def _prepare_expressions_for_pipeline(self, expression, target, count):
7174
replacements[sub_expr] = replacing_expr
7275
return replacements, group
7376

74-
def _prepare_annotations_for_group_pipeline(self):
77+
def _prepare_annotations_for_group_pipeline(self, order_by):
7578
"""Prepare annotations for the MongoDB aggregation pipeline."""
7679
replacements = {}
7780
group = {}
@@ -84,6 +87,14 @@ def _prepare_annotations_for_group_pipeline(self):
8487
replacements.update(new_replacements)
8588
group.update(expr_group)
8689

90+
for expr, _ in order_by:
91+
if expr.contains_aggregate:
92+
new_replacements, expr_group = self._prepare_expressions_for_pipeline(
93+
expr, None, count
94+
)
95+
replacements.update(new_replacements)
96+
group.update(expr_group)
97+
8798
having_replacements, having_group = self._prepare_expressions_for_pipeline(
8899
self.having, None, count
89100
)
@@ -95,9 +106,10 @@ def _get_group_id_expressions(self, order_by):
95106
"""Generate group ID expressions for the aggregation pipeline."""
96107
group_expressions = set()
97108
replacements = {}
98-
for expr, (_, _, is_ref) in order_by:
99-
if not is_ref:
100-
group_expressions |= set(expr.get_group_by_cols())
109+
if not self._meta_ordering:
110+
for expr, (_, _, is_ref) in order_by:
111+
if not is_ref:
112+
group_expressions |= set(expr.get_group_by_cols())
101113

102114
for expr, *_ in self.select:
103115
group_expressions |= set(expr.get_group_by_cols())
@@ -169,7 +181,7 @@ def _build_group_pipeline(self, ids, group):
169181

170182
def pre_sql_setup(self, with_col_aliases=False):
171183
extra_select, order_by, group_by = super().pre_sql_setup(with_col_aliases=with_col_aliases)
172-
group, all_replacements = self._prepare_annotations_for_group_pipeline()
184+
group, all_replacements = self._prepare_annotations_for_group_pipeline(order_by)
173185

174186
# The query.group_by is either None (no GROUP BY at all), True
175187
# (group by select fields), or a list of expressions to be added
@@ -190,6 +202,7 @@ def pre_sql_setup(self, with_col_aliases=False):
190202
)
191203
self._group_pipeline = pipeline
192204

205+
self._order_by = [expr.replace_expressions(all_replacements) for expr, _ in order_by]
193206
self.annotations = {
194207
target: expr.replace_expressions(all_replacements)
195208
for target, expr in self.query.annotation_select.items()
@@ -320,8 +333,13 @@ def build_query(self, columns=None):
320333
query = self.query_class(self)
321334
query.aggregation_pipeline = self.get_aggregation_pipeline()
322335
query.lookup_pipeline = self.get_lookup_pipeline()
323-
query.order_by(self._get_ordering())
324-
query.project_fields = self.get_project_fields(columns, ordering=query.ordering)
336+
ordering_fields, order, need_extra_fields = self.preprocess_orderby()
337+
query.project_fields = self.get_project_fields(columns, ordering_fields)
338+
query.ordering = order
339+
if need_extra_fields and columns is None:
340+
query.extra_fields = self.get_project_fields(
341+
((name, field) for name, field in ordering_fields if name.startswith("__order"))
342+
)
325343
try:
326344
where = getattr(self, "where", self.query.where)
327345
query.mongo_query = (
@@ -367,52 +385,6 @@ def project_field(column):
367385
+ tuple(map(project_field, related_columns))
368386
)
369387

370-
def _get_ordering(self):
371-
"""
372-
Return a list of (field, ascending) tuples that the query results
373-
should be ordered by. If there is no field ordering defined, return
374-
the standard_ordering (a boolean, needed for MongoDB "$natural"
375-
ordering).
376-
"""
377-
opts = self.query.get_meta()
378-
ordering = (
379-
self.query.order_by or opts.ordering
380-
if self.query.default_ordering
381-
else self.query.order_by
382-
)
383-
if not ordering:
384-
return self.query.standard_ordering
385-
default_order, _ = ORDER_DIR["ASC" if self.query.standard_ordering else "DESC"]
386-
column_ordering = []
387-
columns_seen = set()
388-
for order in ordering:
389-
if order == "?":
390-
raise NotSupportedError("Randomized ordering isn't supported by MongoDB.")
391-
if hasattr(order, "resolve_expression"):
392-
# order is an expression like OrderBy, F, or database function.
393-
orderby = order if isinstance(order, OrderBy) else order.asc()
394-
orderby = orderby.resolve_expression(self.query, allow_joins=True, reuse=None)
395-
ascending = not orderby.descending
396-
# If the query is reversed, ascending and descending are inverted.
397-
if not self.query.standard_ordering:
398-
ascending = not ascending
399-
else:
400-
# order is a string like "field" or "field__other_field".
401-
orderby, _ = self.find_ordering_name(
402-
order, self.query.get_meta(), default_order=default_order
403-
)[0]
404-
ascending = not orderby.descending
405-
column = orderby.expression.as_mql(self, self.connection)
406-
if isinstance(column, dict):
407-
raise NotSupportedError("order_by() expression not supported.")
408-
# $sort references must not include the dollar sign.
409-
column = column.removeprefix("$")
410-
# Don't add the same column twice.
411-
if column not in columns_seen:
412-
columns_seen.add(column)
413-
column_ordering.append((column, ascending))
414-
return column_ordering
415-
416388
@cached_property
417389
def collection_name(self):
418390
return self.query.get_meta().db_table
@@ -464,13 +436,31 @@ def get_project_fields(self, columns=None, ordering=None):
464436
if self.query.alias_refcount[alias] and self.collection_name != alias:
465437
fields[alias] = 1
466438

467-
for column, _ in ordering or []:
468-
foreign_table = column.split(".", 1)[0] if "." in column else None
469-
if column not in fields and foreign_table not in fields:
470-
fields[column] = 1
439+
# Add order_by() fields.
440+
for alias, expression in ordering or []:
441+
nested_entity = alias.split(".", 1)[0] if "." in alias else None
442+
if alias not in fields and nested_entity not in fields:
443+
fields[alias] = expression.as_mql(self, self.connection)
471444

472445
return fields
473446

447+
def preprocess_orderby(self):
448+
fields = {}
449+
result = SON()
450+
need_extra_fields = False
451+
idx = itertools.count(start=1)
452+
for order in self._order_by or []:
453+
if isinstance(order.expression, Ref):
454+
fieldname = order.expression.refs
455+
elif isinstance(order.expression, Col):
456+
fieldname = order.expression.as_mql(self, self.connection).removeprefix("$")
457+
else:
458+
fieldname = f"__order{next(idx)}"
459+
need_extra_fields = True
460+
fields[fieldname] = order.expression
461+
result[fieldname] = DESCENDING if order.descending else ASCENDING
462+
return tuple(fields.items()), result, need_extra_fields
463+
474464

475465
class SQLInsertCompiler(SQLCompiler):
476466
def execute_sql(self, returning_fields=None):

django_mongodb/features.py

Lines changed: 4 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -32,35 +32,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
3232
"lookup.tests.LookupTests.test_exact_none_transform",
3333
# "Save with update_fields did not affect any rows."
3434
"basic.tests.SelectOnSaveTests.test_select_on_save_lying_update",
35-
# Order by constant not supported:
36-
# AttributeError: 'Field' object has no attribute 'model'
37-
"aggregation.tests.AggregateTestCase.test_annotate_values_list",
38-
"aggregation.tests.AggregateTestCase.test_combine_different_types",
39-
"aggregation.tests.AggregateTestCase.test_grouped_annotation_in_group_by",
40-
"aggregation.tests.AggregateTestCase.test_non_grouped_annotation_not_in_group_by",
41-
"aggregation.tests.AggregateTestCase.test_values_annotation_with_expression",
42-
"annotations.tests.NonAggregateAnnotationTestCase.test_order_by_aggregate",
43-
"model_fields.test_jsonfield.TestQuerying.test_ordering_grouping_by_count",
44-
"ordering.tests.OrderingTests.test_default_ordering_does_not_affect_group_by",
45-
"ordering.tests.OrderingTests.test_order_by_constant_value",
46-
"expressions.tests.NegatedExpressionTests.test_filter",
47-
"expressions_case.tests.CaseExpressionTests.test_order_by_conditional_implicit",
48-
# NotSupportedError: order_by() expression not supported.
49-
"aggregation.tests.AggregateTestCase.test_aggregation_order_by_not_selected_annotation_values",
50-
"db_functions.comparison.test_coalesce.CoalesceTests.test_ordering",
51-
"db_functions.tests.FunctionTests.test_nested_function_ordering",
52-
"db_functions.text.test_length.LengthTests.test_ordering",
53-
"db_functions.text.test_strindex.StrIndexTests.test_order_by",
54-
"expressions_case.tests.CaseExpressionTests.test_order_by_conditional_explicit",
55-
"lookup.tests.LookupQueryingTests.test_lookup_in_order_by",
56-
"ordering.tests.OrderingTests.test_order_by_expr_query_reuse",
57-
"ordering.tests.OrderingTests.test_order_by_expression_ref",
58-
"ordering.tests.OrderingTests.test_ordering_select_related_collision",
59-
"queries.tests.Queries1Tests.test_order_by_related_field_transform",
60-
"update.tests.AdvancedTests.test_update_ordered_by_inline_m2m_annotation",
61-
"update.tests.AdvancedTests.test_update_ordered_by_m2m_annotation",
62-
"update.tests.AdvancedTests.test_update_ordered_by_m2m_annotation_desc",
63-
# Pattern lookups that use regexMatch don't work on JSONField:
6435
# Unsupported conversion from array to string in $convert
6536
"model_fields.test_jsonfield.TestQuerying.test_icontains",
6637
# MongoDB gives the wrong result of log(number, base) when base is a
@@ -75,11 +46,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
7546
"db_functions.datetime.test_extract_trunc.DateFunctionWithTimeZoneTests.test_trunc_timezone_applied_before_truncation",
7647
# Length of null considered zero rather than null.
7748
"db_functions.text.test_length.LengthTests.test_basic",
78-
# Key transforms are incorrectly treated as joins:
79-
# Ordering can't span tables on MongoDB (value_custom__a).
80-
"model_fields.test_jsonfield.TestQuerying.test_order_grouping_custom_decoder",
81-
"model_fields.test_jsonfield.TestQuerying.test_ordering_by_transform",
82-
"model_fields.test_jsonfield.TestQuerying.test_ordering_grouping_by_key_transform",
8349
# Wrong results in queries with multiple tables.
8450
"annotations.tests.NonAggregateAnnotationTestCase.test_annotation_reverse_m2m",
8551
"annotations.tests.NonAggregateAnnotationTestCase.test_annotation_with_m2m",
@@ -88,30 +54,19 @@ class DatabaseFeatures(BaseDatabaseFeatures):
8854
"expressions.test_queryset_values.ValuesExpressionsTests.test_values_list_expression",
8955
"expressions.test_queryset_values.ValuesExpressionsTests.test_values_list_expression_flat",
9056
"expressions.tests.IterableLookupInnerExpressionsTests.test_expressions_in_lookups_join_choice",
91-
"ordering.tests.OrderingTests.test_order_by_grandparent_fk_with_expression_in_default_ordering",
92-
"ordering.tests.OrderingTests.test_order_by_parent_fk_with_expression_in_default_ordering",
93-
"ordering.tests.OrderingTests.test_order_by_ptr_field_with_default_ordering_by_expression",
9457
"queries.tests.Queries1Tests.test_order_by_tables",
9558
"queries.tests.TestTicket24605.test_ticket_24605",
9659
"queries.tests.TestInvalidValuesRelation.test_invalid_values",
97-
# alias().order_by() doesn't work.
98-
"annotations.tests.AliasTests.test_order_by_alias",
99-
"annotations.tests.AliasTests.test_order_by_alias_aggregate",
100-
# annotate() + values_list() + order_by() loses annotated value.
101-
"expressions_case.tests.CaseExpressionTests.test_annotate_values_not_in_order_by",
10260
# QuerySet.explain() not implemented:
10361
# https://github.com/mongodb-labs/django-mongodb/issues/28
10462
"queries.test_explain.ExplainUnsupportedTests.test_message",
105-
# Sum returns 0 instead of None in mongodb.
63+
# Sum returns 0 instead of None in MongoDB.
10664
"aggregation.test_filter_argument.FilteredAggregateTests.test_plain_annotate",
10765
"aggregation.tests.AggregateTestCase.test_aggregation_default_passed_another_aggregate",
10866
"aggregation.tests.AggregateTestCase.test_annotation_expressions",
10967
"aggregation.tests.AggregateTestCase.test_reverse_fkey_annotate",
11068
# Manage empty result when the flag elide_empty is False
11169
"aggregation.tests.AggregateTestCase.test_empty_result_optimization",
112-
# Incorrect order: pipeline does not order by the right fields.
113-
"aggregation.tests.AggregateTestCase.test_annotate_ordering",
114-
"aggregation.tests.AggregateTestCase.test_even_more_aggregate",
11570
}
11671
# $bitAnd, #bitOr, and $bitXor are new in MongoDB 6.3.
11772
_django_test_expected_failures_bitwise = {
@@ -398,6 +353,8 @@ def django_test_expected_failures(self):
398353
"Cannot use QuerySet.update() when querying across multiple collections on MongoDB.": {
399354
"queries.tests.Queries4Tests.test_ticket7095",
400355
"queries.tests.Queries5Tests.test_ticket9848",
356+
"update.tests.AdvancedTests.test_update_ordered_by_m2m_annotation",
357+
"update.tests.AdvancedTests.test_update_ordered_by_m2m_annotation_desc",
401358
},
402359
"QuerySet.dates() is not supported on MongoDB.": {
403360
"aggregation.tests.AggregateTestCase.test_dates_with_aggregation",
@@ -494,6 +451,7 @@ def django_test_expected_failures(self):
494451
"model_fields.test_jsonfield.TestQuerying.test_key_transform_raw_expression",
495452
"model_fields.test_jsonfield.TestQuerying.test_nested_key_transform_raw_expression",
496453
"queries.tests.Queries1Tests.test_order_by_rawsql",
454+
"queries.tests.Queries5Tests.test_ordering",
497455
"timezones.tests.LegacyDatabaseTests.test_cursor_execute_accepts_naive_datetime",
498456
"timezones.tests.LegacyDatabaseTests.test_cursor_execute_returns_naive_datetime",
499457
"timezones.tests.LegacyDatabaseTests.test_raw_sql",
@@ -576,10 +534,6 @@ def django_test_expected_failures(self):
576534
"model_fields.test_jsonfield.TestQuerying.test_none_key",
577535
"model_fields.test_jsonfield.TestQuerying.test_none_key_exclude",
578536
},
579-
"Randomized ordering isn't supported by MongoDB.": {
580-
"aggregation.tests.AggregateTestCase.test_aggregation_random_ordering",
581-
"ordering.tests.OrderingTests.test_random_ordering",
582-
},
583537
"Queries without a collection aren't supported on MongoDB.": {
584538
"queries.test_q.QCheckTests",
585539
"queries.test_query.TestQueryNoModel",

django_mongodb/operations.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,11 @@ def convert_floatfield_value(self, value, expression, connection):
135135
return float(value)
136136

137137
def convert_integerfield_value(self, value, expression, connection):
138-
return None if value is None else int(value)
138+
if value is None:
139+
return value
140+
if isinstance(value, Decimal128):
141+
return int(value.to_decimal())
142+
return int(value)
139143

140144
def convert_jsonfield_value(self, value, expression, connection):
141145
"""

django_mongodb/query.py

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from django.db.models.sql.constants import INNER
1010
from django.db.models.sql.datastructures import Join
1111
from django.db.models.sql.where import AND, OR, XOR, WhereNode
12-
from pymongo import ASCENDING, DESCENDING
1312
from pymongo.errors import DuplicateKeyError, PyMongoError
1413

1514

@@ -51,28 +50,11 @@ def __init__(self, compiler):
5150
self.lookup_pipeline = None
5251
self.project_fields = None
5352
self.aggregation_pipeline = None
53+
self.extra_fields = None
5454

5555
def __repr__(self):
5656
return f"<MongoQuery: {self.mongo_query!r} ORDER {self.ordering!r}>"
5757

58-
def order_by(self, ordering):
59-
"""
60-
Reorder query results or execution order. Called by compiler during
61-
query building.
62-
63-
`ordering` is a list with (column, ascending) tuples or a boolean --
64-
use natural ordering, if any, when the argument is True and its reverse
65-
otherwise.
66-
"""
67-
if isinstance(ordering, bool):
68-
# No need to add {$natural: ASCENDING} as it's the default.
69-
if not ordering:
70-
self.ordering.append(("$natural", DESCENDING))
71-
else:
72-
for column, ascending in ordering:
73-
direction = ASCENDING if ascending else DESCENDING
74-
self.ordering.append((column, direction))
75-
7658
@wrap_database_errors
7759
def delete(self):
7860
"""Execute a delete query."""
@@ -101,6 +83,8 @@ def get_pipeline(self):
10183
pipeline.extend(self.aggregation_pipeline)
10284
if self.project_fields:
10385
pipeline.append({"$project": self.project_fields})
86+
if self.extra_fields:
87+
pipeline.append({"$addFields": self.extra_fields})
10488
if self.ordering:
10589
pipeline.append({"$sort": dict(self.ordering)})
10690
if self.query.low_mark > 0:

0 commit comments

Comments
 (0)