From c2d4926702045e342a668057f0a758eec9db9436 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Tue, 3 Nov 2020 16:50:10 -0500 Subject: Fixed #31910 -- Fixed crash of GIS aggregations over subqueries. Regression was introduced by fff5186 but was due a long standing issue. AggregateQuery was abusing Query.subquery: bool by stashing its compiled inner query's SQL for later use in its compiler which made select_format checks for Query.subquery wrongly assume the provide query was a subquery. This patch prevents that from happening by using a dedicated inner_query attribute which is compiled at a later time by SQLAggregateCompiler. Moving the inner query's compilation to SQLAggregateCompiler.compile had the side effect of addressing a long standing issue with aggregation subquery pushdown which prevented converters from being run. This is now fixed as the aggregation_regress adjustments demonstrate. Refs #25367. Thanks Eran Keydar for the report. --- django/db/models/sql/compiler.py | 7 +++++-- django/db/models/sql/query.py | 14 +++----------- django/db/models/sql/subqueries.py | 6 +++--- 3 files changed, 11 insertions(+), 16 deletions(-) (limited to 'django/db/models/sql') diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 04e430a42e..3360f9c806 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -1596,8 +1596,11 @@ class SQLAggregateCompiler(SQLCompiler): sql = ', '.join(sql) params = tuple(params) - sql = 'SELECT %s FROM (%s) subquery' % (sql, self.query.subquery) - params = params + self.query.sub_params + inner_query_sql, inner_query_params = self.query.inner_query.get_compiler( + self.using + ).as_sql(with_col_aliases=True) + sql = 'SELECT %s FROM (%s) subquery' % (sql, inner_query_sql) + params = params + inner_query_params return sql, params diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index d34b9da601..a7dadf5a40 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -17,9 +17,7 @@ from collections.abc import Iterator, Mapping from itertools import chain, count, product from string import ascii_uppercase -from django.core.exceptions import ( - EmptyResultSet, FieldDoesNotExist, FieldError, -) +from django.core.exceptions import FieldDoesNotExist, FieldError from django.db import DEFAULT_DB_ALIAS, NotSupportedError, connections from django.db.models.aggregates import Count from django.db.models.constants import LOOKUP_SEP @@ -449,8 +447,9 @@ class Query(BaseExpression): if (isinstance(self.group_by, tuple) or self.is_sliced or existing_annotations or self.distinct or self.combinator): from django.db.models.sql.subqueries import AggregateQuery - outer_query = AggregateQuery(self.model) inner_query = self.clone() + inner_query.subquery = True + outer_query = AggregateQuery(self.model, inner_query) inner_query.select_for_update = False inner_query.select_related = False inner_query.set_annotation_mask(self.annotation_select) @@ -492,13 +491,6 @@ class Query(BaseExpression): # field selected in the inner query, yet we must use a subquery. # So, make sure at least one field is selected. inner_query.select = (self.model._meta.pk.get_col(inner_query.get_initial_alias()),) - try: - outer_query.add_subquery(inner_query, using) - except EmptyResultSet: - return { - alias: None - for alias in outer_query.annotation_select - } else: outer_query = self self.select = () diff --git a/django/db/models/sql/subqueries.py b/django/db/models/sql/subqueries.py index 72b6712864..e83112b046 100644 --- a/django/db/models/sql/subqueries.py +++ b/django/db/models/sql/subqueries.py @@ -157,6 +157,6 @@ class AggregateQuery(Query): compiler = 'SQLAggregateCompiler' - def add_subquery(self, query, using): - query.subquery = True - self.subquery, self.sub_params = query.get_compiler(using).as_sql(with_col_aliases=True) + def __init__(self, model, inner_query): + self.inner_query = inner_query + super().__init__(model) -- cgit v1.3