From d8c90d4c22cb218f1c170eba086c53d3dff7fbc0 Mon Sep 17 00:00:00 2001 From: Hannes Ljungberg Date: Tue, 25 May 2021 21:17:05 +0200 Subject: [PATCH] Fixed #32786 -- Moved subquery ordering clearing optimization to the _in lookup. Co-Authored-By: Simon Charette --- django/db/backends/base/features.py | 3 +++ django/db/backends/oracle/features.py | 1 + django/db/models/lookups.py | 13 ++++++++++--- django/db/models/sql/query.py | 13 +++++++------ django/db/models/sql/where.py | 1 + tests/postgres_tests/test_aggregates.py | 19 ++++++++++++++++++- 6 files changed, 40 insertions(+), 10 deletions(-) diff --git a/django/db/backends/base/features.py b/django/db/backends/base/features.py index 4c892312950..c9964d52290 100644 --- a/django/db/backends/base/features.py +++ b/django/db/backends/base/features.py @@ -64,6 +64,9 @@ class BaseDatabaseFeatures: has_real_datatype = False supports_subqueries_in_group_by = True + # Does the backend ignore unnecessary ORDER BY clauses in subqueries? + ignores_unnecessary_order_by_in_subqueries = True + # Is there a true datatype for uuid? has_native_uuid_field = False diff --git a/django/db/backends/oracle/features.py b/django/db/backends/oracle/features.py index 07f16eee71d..2d29b590152 100644 --- a/django/db/backends/oracle/features.py +++ b/django/db/backends/oracle/features.py @@ -15,6 +15,7 @@ class DatabaseFeatures(BaseDatabaseFeatures): select_for_update_of_column = True can_return_columns_from_insert = True supports_subqueries_in_group_by = False + ignores_unnecessary_order_by_in_subqueries = False supports_transactions = True supports_timezones = False has_native_duration_field = True diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index defa566f215..8eb6702204b 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -404,9 +404,16 @@ class In(FieldGetDbPrepValueIterableMixin, BuiltinLookup): placeholder = '(' + ', '.join(sqls) + ')' return (placeholder, sqls_params) else: - if not getattr(self.rhs, 'has_select_fields', True): - self.rhs.clear_select_clause() - self.rhs.add_fields(['pk']) + from django.db.models.sql.query import ( # avoid circular import + Query, + ) + if isinstance(self.rhs, Query): + query = self.rhs + query.clear_ordering(clear_default=True) + if not query.has_select_fields: + query.clear_select_clause() + query.add_fields(['pk']) + return super().process_rhs(compiler, connection) def get_group_by_cols(self, alias=None): diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 11c83bfe1a4..814271a1f6e 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1034,12 +1034,6 @@ class Query(BaseExpression): # Subqueries need to use a different set of aliases than the outer query. clone.bump_prefix(query) clone.subquery = True - # It's safe to drop ordering if the queryset isn't using slicing, - # distinct(*fields) or select_for_update(). - 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(force=True) clone.where.resolve_expression(query, *args, **kwargs) for key, value in clone.annotations.items(): resolved = value.resolve_expression(query, *args, **kwargs) @@ -1062,6 +1056,13 @@ class Query(BaseExpression): ] def as_sql(self, compiler, connection): + # Some backends (e.g. Oracle) raise an error when a subquery contains + # unnecessary ORDER BY clause. + if ( + self.subquery and + not connection.features.ignores_unnecessary_order_by_in_subqueries + ): + self.clear_ordering(force=False) sql, params = self.get_compiler(connection=connection).as_sql() if self.subquery: sql = '(%s)' % sql diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py index 795eff83950..2577e1d7a50 100644 --- a/django/db/models/sql/where.py +++ b/django/db/models/sql/where.py @@ -239,6 +239,7 @@ class SubqueryConstraint: self.alias = alias self.columns = columns self.targets = targets + query_object.clear_ordering(clear_default=True) self.query_object = query_object def as_sql(self, compiler, connection): diff --git a/tests/postgres_tests/test_aggregates.py b/tests/postgres_tests/test_aggregates.py index 7f21def1bed..d47c24203b7 100644 --- a/tests/postgres_tests/test_aggregates.py +++ b/tests/postgres_tests/test_aggregates.py @@ -1,4 +1,6 @@ -from django.db.models import CharField, F, OuterRef, Q, Subquery, Value +from django.db.models import ( + CharField, F, Func, IntegerField, OuterRef, Q, Subquery, Value, +) from django.db.models.fields.json import KeyTextTransform, KeyTransform from django.db.models.functions import Cast, Concat, Substr from django.test.utils import Approximate @@ -12,6 +14,7 @@ try: RegrAvgX, RegrAvgY, RegrCount, RegrIntercept, RegrR2, RegrSlope, RegrSXX, RegrSXY, RegrSYY, StatAggregate, StringAgg, ) + from django.contrib.postgres.fields import ArrayField except ImportError: pass # psycopg2 is not installed @@ -390,6 +393,20 @@ class TestGeneralAggregate(PostgreSQLTestCase): [self.aggs[0]], ) + def test_ordering_isnt_cleared_for_array_subquery(self): + inner_qs = AggregateTestModel.objects.order_by('-integer_field') + qs = AggregateTestModel.objects.annotate( + integers=Func( + Subquery(inner_qs.values('integer_field')), + function='ARRAY', + output_field=ArrayField(base_field=IntegerField()), + ), + ) + self.assertSequenceEqual( + qs.first().integers, + inner_qs.values_list('integer_field', flat=True), + ) + class TestAggregateDistinct(PostgreSQLTestCase): @classmethod