From 053141d31fe5aef1c255a1be183383860e0ccce9 Mon Sep 17 00:00:00 2001 From: Hannes Ljungberg Date: Wed, 26 May 2021 22:09:15 +0200 Subject: [PATCH] Refs #32786 -- Made Query.clear_ordering() not to cause side effects by default. --- 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 f14ff8d094..7856cf1a9f 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -658,7 +658,7 @@ class QuerySet: ) 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() @@ -741,7 +741,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) @@ -1009,7 +1009,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 @@ -1166,7 +1166,7 @@ class QuerySet: if self.query.is_sliced: raise TypeError('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 7264929da8..6082017bbd 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1609,7 +1609,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 6629a1fd51..11c83bfe1a 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -447,11 +447,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 @@ -491,7 +490,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 @@ -534,7 +533,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) @@ -1040,7 +1039,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) @@ -1769,7 +1768,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) @@ -1970,14 +1969,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):