Skip to content

Commit

Permalink
[3.0.x] Fixed #31060 -- Reallowed window expressions to be used in co…
Browse files Browse the repository at this point in the history
…nditions outside of queryset filters.

Regression in 4edad1d.

Thanks utapyngo for the report.

Backport of bf12273 from master.
  • Loading branch information
gizmondo authored and felixxm committed Dec 6, 2019
1 parent e986e49 commit 8af0771
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 8 deletions.
5 changes: 4 additions & 1 deletion django/db/models/query_utils.py
Expand Up @@ -90,7 +90,10 @@ def __invert__(self):
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

Expand Down
16 changes: 11 additions & 5 deletions django/db/models/sql/query.py
Expand Up @@ -1192,7 +1192,8 @@ def try_transform(self, lhs, name):

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, simple_col=False):
reuse_with_filtered_relation=False, simple_col=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
Expand Down Expand Up @@ -1238,7 +1239,8 @@ def build_filter(self, filter_expr, branch_negated=False, current_negated=False,
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")
Expand All @@ -1247,7 +1249,8 @@ def build_filter(self, filter_expr, branch_negated=False, current_negated=False,
value = self.resolve_lookup_value(value, can_reuse, allow_joins, simple_col)
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:
Expand Down Expand Up @@ -1344,7 +1347,7 @@ def build_where(self, q_object):

def _add_q(self, q_object, used_aliases, branch_negated=False,
current_negated=False, allow_joins=True, split_subq=True,
simple_col=False):
simple_col=False, check_filterable=True):
"""Add a Q-object to the current filter."""
connector = q_object.connector
current_negated = current_negated ^ q_object.negated
Expand All @@ -1356,13 +1359,16 @@ def _add_q(self, q_object, used_aliases, branch_negated=False,
if isinstance(child, Node):
child_clause, needed_inner = self._add_q(
child, used_aliases, branch_negated,
current_negated, allow_joins, split_subq, simple_col)
current_negated, allow_joins, split_subq, simple_col,
check_filterable,
)
joinpromoter.add_votes(needed_inner)
else:
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, simple_col=simple_col,
check_filterable=check_filterable,
)
joinpromoter.add_votes(needed_inner)
if child_clause:
Expand Down
5 changes: 5 additions & 0 deletions docs/releases/3.0.1.txt
Expand Up @@ -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`).
20 changes: 18 additions & 2 deletions tests/expressions_window/tests.py
Expand Up @@ -4,8 +4,8 @@
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 (
Expand Down Expand Up @@ -846,6 +846,22 @@ def test_invalid_filter(self):
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):
Expand Down

0 comments on commit 8af0771

Please sign in to comment.