From: Hannes Ljungberg Date: Wed, 26 May 2021 22:09:15 +0200 Subject: Refs #32786 -- Made Query.clear_ordering() not to cause side effects by default. (cherry picked from commit 053141d31fe5aef1c255a1be183383860e0ccce9) --- django/db/models/query.py | 8 ++++---- django/db/models/sql/compiler.py | 2 +- django/db/models/sql/query.py | 29 ++++++++++++++++------------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 02c1b314355c..5ba0a10956c6 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -659,7 +659,7 @@ class QuerySet: "Cannot change a query once a slice has been taken." obj = self._chain() obj.query.set_limits(high=1) - obj.query.clear_ordering(force_empty=True) + obj.query.clear_ordering(force=True) obj.query.add_ordering(*order_by) return obj.get() @@ -739,7 +739,7 @@ class QuerySet: # Disable non-supported fields. del_query.query.select_for_update = False del_query.query.select_related = False - del_query.query.clear_ordering(force_empty=True) + del_query.query.clear_ordering(force=True) collector = Collector(using=del_query.db) collector.collect(del_query) @@ -988,7 +988,7 @@ class QuerySet: # Clone the query to inherit the select list and everything clone = self._chain() # Clear limits and ordering so they can be reapplied - clone.query.clear_ordering(True) + clone.query.clear_ordering(force=True) clone.query.clear_limits() clone.query.combined_queries = (self.query,) + tuple(qs.query for qs in other_qs) clone.query.combinator = combinator @@ -1145,7 +1145,7 @@ class QuerySet: assert not self.query.is_sliced, \ "Cannot reorder a query once a slice has been taken." obj = self._chain() - obj.query.clear_ordering(force_empty=False) + obj.query.clear_ordering(force=True, clear_default=False) obj.query.add_ordering(*field_names) return obj diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 6254946bc43f..a55e1d3c363c 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1587,7 +1587,7 @@ class SQLUpdateCompiler(SQLCompiler): return query = self.query.chain(klass=Query) query.select_related = False - query.clear_ordering(True) + query.clear_ordering(force=True) query.extra = {} query.select = [] query.add_fields([query.get_meta().pk.name]) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 230b6fa8610e..db52a4976951 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -461,11 +461,10 @@ class Query(BaseExpression): inner_query.select_for_update = False inner_query.select_related = False inner_query.set_annotation_mask(self.annotation_select) - if not self.is_sliced and not self.distinct_fields: - # Queries with distinct_fields need ordering and when a limit - # is applied we must take the slice from the ordered query. - # Otherwise no need for ordering. - inner_query.clear_ordering(True) + # Queries with distinct_fields need ordering and when a limit is + # applied we must take the slice from the ordered query. Otherwise + # no need for ordering. + inner_query.clear_ordering(force=False) if not inner_query.distinct: # If the inner query uses default select and it has some # aggregate annotations, then we must make sure the inner @@ -505,7 +504,7 @@ class Query(BaseExpression): self.default_cols = False self.extra = {} - outer_query.clear_ordering(True) + outer_query.clear_ordering(force=True) outer_query.clear_limits() outer_query.select_for_update = False outer_query.select_related = False @@ -548,7 +547,7 @@ class Query(BaseExpression): combined_query.exists(using, limit=limit_combined) for combined_query in q.combined_queries ) - q.clear_ordering(True) + q.clear_ordering(force=True) if limit: q.set_limits(high=1) q.add_extra({'a': 1}, None, None, None, None, None) @@ -1077,7 +1076,7 @@ class Query(BaseExpression): if (self.low_mark == 0 and self.high_mark is None and not self.distinct_fields and not self.select_for_update): - clone.clear_ordering(True) + clone.clear_ordering(force=True) clone.where.resolve_expression(query, *args, **kwargs) for key, value in clone.annotations.items(): resolved = value.resolve_expression(query, *args, **kwargs) @@ -1818,7 +1817,7 @@ class Query(BaseExpression): query = Query(self.model) query._filtered_relations = self._filtered_relations query.add_filter(filter_expr) - query.clear_ordering(True) + query.clear_ordering(force=True) # Try to have as simple as possible subquery -> trim leading joins from # the subquery. trimmed_prefix, contains_louter = query.trim_start(names_with_path) @@ -2028,14 +2027,18 @@ class Query(BaseExpression): else: self.default_ordering = False - def clear_ordering(self, force_empty): + def clear_ordering(self, force=False, clear_default=True): """ - Remove any ordering settings. If 'force_empty' is True, there will be - no ordering in the resulting query (not even the model's default). + Remove any ordering settings if the current query allows it without + side effects, set 'force' to True to clear the ordering regardless. + If 'clear_default' is True, there will be no ordering in the resulting + query (not even the model's default). """ + if not force and (self.is_sliced or self.distinct_fields or self.select_for_update): + return self.order_by = () self.extra_order_by = () - if force_empty: + if clear_default: self.default_ordering = False def set_group_by(self, allow_aliases=True):