From 3a505c70e7b228bf1212c067a8f38271ca86ce09 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Wed, 6 Mar 2019 01:24:41 -0500 Subject: [PATCH] Refs #27149, #29542 -- Simplified subquery parentheses wrapping logic. --- django/db/models/expressions.py | 14 ++------------ django/db/models/lookups.py | 3 +-- django/db/models/sql/compiler.py | 7 +------ django/db/models/sql/query.py | 5 ++++- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/django/db/models/expressions.py b/django/db/models/expressions.py index 7b703c8f1c..5d630a63b3 100644 --- a/django/db/models/expressions.py +++ b/django/db/models/expressions.py @@ -1023,23 +1023,13 @@ class Subquery(Expression): def as_sql(self, compiler, connection, template=None, **extra_context): connection.ops.check_expression_support(self) template_params = {**self.extra, **extra_context} - template_params['subquery'], sql_params = self.query.as_sql(compiler, connection) + subquery_sql, sql_params = self.query.as_sql(compiler, connection) + template_params['subquery'] = subquery_sql[1:-1] template = template or template_params.get('template', self.template) sql = template % template_params return sql, sql_params - def _prepare(self, output_field): - # This method will only be called if this instance is the "rhs" in an - # expression: the wrapping () must be removed (as the expression that - # contains this will provide them). SQLite evaluates ((subquery)) - # differently than the other databases. - if self.template == '(%(subquery)s)': - clone = self.copy() - clone.template = '%(subquery)s' - return clone - return self - def get_group_by_cols(self, alias=None): if alias: return [Ref(alias, self)] diff --git a/django/db/models/lookups.py b/django/db/models/lookups.py index e78ffdf390..910291094e 100644 --- a/django/db/models/lookups.py +++ b/django/db/models/lookups.py @@ -89,8 +89,7 @@ class Lookup: value = self.apply_bilateral_transforms(value) value = value.resolve_expression(compiler.query) if hasattr(value, 'as_sql'): - sql, params = compiler.compile(value) - return '(' + sql + ')', params + return compiler.compile(value) else: return self.get_db_prep_lookup(value, connection) diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 626d095624..f4b1faabc2 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -5,7 +5,7 @@ from itertools import chain from django.core.exceptions import EmptyResultSet, FieldError from django.db.models.constants import LOOKUP_SEP -from django.db.models.expressions import OrderBy, Random, RawSQL, Ref, Subquery +from django.db.models.expressions import OrderBy, Random, RawSQL, Ref from django.db.models.query_utils import QueryWrapper, select_related_descend from django.db.models.sql.constants import ( CURSOR, GET_ITERATOR_CHUNK_SIZE, MULTI, NO_RESULTS, ORDER_DIR, SINGLE, @@ -126,11 +126,6 @@ class SQLCompiler: for expr in expressions: sql, params = self.compile(expr) - if isinstance(expr, Subquery) and not sql.startswith('('): - # Subquery expression from HAVING clause may not contain - # wrapping () because they could be removed when a subquery is - # the "rhs" in an expression (see Subquery._prepare()). - sql = '(%s)' % sql if (sql, tuple(params)) not in seen: result.append((sql, params)) seen.add((sql, tuple(params))) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index c192530573..3aa3c0b512 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1022,7 +1022,10 @@ class Query(BaseExpression): return clone def as_sql(self, compiler, connection): - return self.get_compiler(connection=connection).as_sql() + sql, params = self.get_compiler(connection=connection).as_sql() + if self.subquery: + sql = '(%s)' % sql + return sql, params def resolve_lookup_value(self, value, can_reuse, allow_joins, simple_col): if hasattr(value, 'resolve_expression'):