From bf12273db4e53779546e2ac7b65c0ce8e3c8a640 Mon Sep 17 00:00:00 2001 From: Alex Aktsipetrov Date: Thu, 5 Dec 2019 19:08:47 +0300 Subject: [PATCH] Fixed #31060 -- Reallowed window expressions to be used in conditions outside of queryset filters. Regression in 4edad1ddf6203326e0be4bdb105beecb0fe454c4. Thanks utapyngo for the report. --- django/db/models/query_utils.py | 5 ++++- django/db/models/sql/query.py | 14 +++++++++----- docs/releases/3.0.1.txt | 5 +++++ tests/expressions_window/tests.py | 20 ++++++++++++++++++-- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py index a6b71545410..96409cc67ce 100644 --- a/django/db/models/query_utils.py +++ b/django/db/models/query_utils.py @@ -100,7 +100,10 @@ class Q(tree.Node): def resolve_expression(self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False): # We must promote any new joins to left outer joins so that when Q is # used as an expression, rows aren't filtered due to joins. - clause, joins = query._add_q(self, reuse, allow_joins=allow_joins, split_subq=False) + clause, joins = query._add_q( + self, reuse, allow_joins=allow_joins, split_subq=False, + check_filterable=False, + ) query.promote_joins(joins) return clause diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 8fee6b01836..af1ea8b84b1 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -1190,7 +1190,7 @@ class Query(BaseExpression): def build_filter(self, filter_expr, branch_negated=False, current_negated=False, can_reuse=None, allow_joins=True, split_subq=True, - reuse_with_filtered_relation=False): + reuse_with_filtered_relation=False, check_filterable=True): """ Build a WhereNode for a single filter clause but don't add it to this Query. Query.add_q() will then add this filter to the where @@ -1229,6 +1229,7 @@ class Query(BaseExpression): used_aliases=can_reuse, allow_joins=allow_joins, split_subq=split_subq, + check_filterable=check_filterable, ) if hasattr(filter_expr, 'resolve_expression'): if not getattr(filter_expr, 'conditional', False): @@ -1244,7 +1245,8 @@ class Query(BaseExpression): raise FieldError("Cannot parse keyword query %r" % arg) lookups, parts, reffed_expression = self.solve_lookup_type(arg) - self.check_filterable(reffed_expression) + if check_filterable: + self.check_filterable(reffed_expression) if not allow_joins and len(parts) > 1: raise FieldError("Joined field references are not permitted in this query") @@ -1253,7 +1255,8 @@ class Query(BaseExpression): value = self.resolve_lookup_value(value, can_reuse, allow_joins) used_joins = {k for k, v in self.alias_refcount.items() if v > pre_joins.get(k, 0)} - self.check_filterable(value) + if check_filterable: + self.check_filterable(value) clause = self.where_class() if reffed_expression: @@ -1349,7 +1352,8 @@ class Query(BaseExpression): return self.build_filter(filter_expr, allow_joins=False)[0] def _add_q(self, q_object, used_aliases, branch_negated=False, - current_negated=False, allow_joins=True, split_subq=True): + current_negated=False, allow_joins=True, split_subq=True, + check_filterable=True): """Add a Q-object to the current filter.""" connector = q_object.connector current_negated = current_negated ^ q_object.negated @@ -1361,7 +1365,7 @@ class Query(BaseExpression): child_clause, needed_inner = self.build_filter( child, can_reuse=used_aliases, branch_negated=branch_negated, current_negated=current_negated, allow_joins=allow_joins, - split_subq=split_subq, + split_subq=split_subq, check_filterable=check_filterable, ) joinpromoter.add_votes(needed_inner) if child_clause: diff --git a/docs/releases/3.0.1.txt b/docs/releases/3.0.1.txt index cdfdc33e40b..e5ec2c70d59 100644 --- a/docs/releases/3.0.1.txt +++ b/docs/releases/3.0.1.txt @@ -17,3 +17,8 @@ Bugfixes * Fixed a regression in Django 3.0 where ``RegexPattern``, used by :func:`~django.urls.re_path`, returned positional arguments to be passed to the view when all optional named groups were missing (:ticket:`31061`). + +* Reallowed, following a regression in Django 3.0, + :class:`~django.db.models.expressions.Window` expressions to be used in + conditions outside of queryset filters, e.g. in + :class:`~django.db.models.expressions.When` conditions (:ticket:`31060`). diff --git a/tests/expressions_window/tests.py b/tests/expressions_window/tests.py index 580d038e29e..4df58bf27dc 100644 --- a/tests/expressions_window/tests.py +++ b/tests/expressions_window/tests.py @@ -4,8 +4,8 @@ from unittest import mock, skipIf, skipUnless from django.core.exceptions import FieldError from django.db import NotSupportedError, connection from django.db.models import ( - F, Func, OuterRef, Q, RowRange, Subquery, Value, ValueRange, Window, - WindowFrame, + BooleanField, Case, F, Func, OuterRef, Q, RowRange, Subquery, Value, + ValueRange, When, Window, WindowFrame, ) from django.db.models.aggregates import Avg, Max, Min, Sum from django.db.models.functions import ( @@ -846,6 +846,22 @@ class NonQueryWindowTests(SimpleTestCase): with self.assertRaisesMessage(NotSupportedError, msg): qs.annotate(total=Sum('dense_rank', filter=Q(name='Jones'))).filter(total=1) + def test_conditional_annotation(self): + qs = Employee.objects.annotate( + dense_rank=Window(expression=DenseRank()), + ).annotate( + equal=Case( + When(id=F('dense_rank'), then=Value(True)), + default=Value(False), + output_field=BooleanField(), + ), + ) + # The SQL standard disallows referencing window functions in the WHERE + # clause. + msg = 'Window is disallowed in the filter clause' + with self.assertRaisesMessage(NotSupportedError, msg): + qs.filter(equal=True) + def test_invalid_order_by(self): msg = 'order_by must be either an Expression or a sequence of expressions' with self.assertRaisesMessage(ValueError, msg):