[Django] #34368: Subquery referencing WINDOW expression breaks query compilation
Django
-------------------------------------+-------------------------------------
Reporter: Jannis | Owner: nobody
Vajen |
Type: Bug | Status: new
Component: Database | Version: 4.2
layer (models, ORM) |
Severity: Release | Keywords: window orm filter
blocker | subquery
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
#28333 introduced the ability to filter on WINDOW expressions but when a
WINDOW expression is referred to in a subquery, the query compiler fails:
`ValueError: Need either using or connection`
I assume it has to do with `self.connection` not being passed on to
`inner_query.get_compiler` in `get_qualify_sql` introduced here:
https://github.com/django/django/commit/f387d024fc75569d2a4a338bfda76cc2f328f627
#diff-f58de2deaccecd2d53199c5ca29e3e1050ec2adb80fb057cdfc0b4e6accdf14fR624
Below is the full traceback. A dummy test is available here:
https://github.com/jnns/django/commit/04d59e27503fb376d60314806876ecbae89d9b62
{{{#!python
Traceback (most recent call last):
File "/usr/lib64/python3.11/unittest/case.py", line 57, in
testPartExecutor
yield
File "/usr/lib64/python3.11/unittest/case.py", line 623, in run
self._callTestMethod(testMethod)
File "/usr/lib64/python3.11/unittest/case.py", line 579, in
_callTestMethod
if method() is not None:
File "tests/expressions_window/tests.py", line 1025, in
test_filter_subquery
list(
File "django/db/models/query.py", line 398, in __iter__
self._fetch_all()
File "django/db/models/query.py", line 1881, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "django/db/models/query.py", line 91, in __iter__
results = compiler.execute_sql(
File "django/db/models/sql/compiler.py", line 1545, in execute_sql
sql, params = self.as_sql()
File "django/db/models/sql/compiler.py", line 732, in as_sql
extra_select, order_by, group_by = self.pre_sql_setup(
File "django/db/models/sql/compiler.py", line 84, in pre_sql_setup
self.setup_query(with_col_aliases=with_col_aliases)
File "django/db/models/sql/compiler.py", line 73, in setup_query
self.select, self.klass_info, self.annotation_col_map =
self.get_select(
File "django/db/models/sql/compiler.py", line 296, in get_select
sql, params = self.compile(col)
File "django/db/models/sql/compiler.py", line 542, in compile
sql, params = node.as_sql(self, self.connection)
File "django/db/models/expressions.py", line 1536, in as_sql
subquery_sql, sql_params = self.query.as_sql(compiler, connection)
File "django/db/models/sql/query.py", line 1150, in as_sql
sql, params = self.get_compiler(connection=connection).as_sql()
File "django/db/models/sql/compiler.py", line 751, in as_sql
result, params = self.get_qualify_sql()
File "django/db/models/sql/compiler.py", line 676, in get_qualify_sql
inner_query_compiler = inner_query.get_compiler(
File "django/db/models/sql/query.py", line 298, in get_compiler
raise ValueError("Need either using or connection")
ValueError: Need either using or connection
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34368>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: window orm filter | Triage Stage: Accepted
subquery |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Simon Charette (added)
* stage: Unreviewed => Accepted
Comment:
Thanks again for the report! Would you like to prepare a patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: window orm filter | Triage Stage: Accepted
subquery |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jannis Vajen):
I started work on https://github.com/django/django/pull/16597 but now I'm
in need of assistance. The query compiles fine if `connection` is added,
but I am unsure about the correctness of the resulting query.
I added the test `WindowFunctionTests.test_filter_subquery` to describe
the behaviour I expected to see. Maybe this is out of scope for the
referenced feature #15922 or completely intentional. I am not sure.
This is the query in the failing test is currently generated:
{{{#!sql
SELECT "expressions_window_employee"."name",
(SELECT "col1"
FROM
(SELECT *
FROM
(SELECT U0."code" AS "col1",
RANK() OVER (PARTITION BY
"expressions_window_employee"."department"
ORDER BY
"expressions_window_employee"."salary" DESC) AS "qual0"
FROM "expressions_window_classification" U0) "qualify"
WHERE "col1" = ("qual0") ) "qualify_mask"
LIMIT 1) AS "code"
FROM "expressions_window_employee"
}}}
Whereas this is what I would expect:
{{{#!sql
SELECT name,
(SELECT "code"
FROM "expressions_window_classification"
WHERE "code" = "department_salary_rank"
LIMIT 1) AS code
FROM (
SELECT
"expressions_window_employee"."name",
RANK() OVER (PARTITION BY "expressions_window_employee"."department"
ORDER BY "expressions_window_employee"."salary" DESC) AS
"department_salary_rank"
FROM "expressions_window_employee"
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: window orm filter | Triage Stage: Accepted
subquery |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Passing `connection` will address the crash but there seems to be a more
fundamental issue when using `OuterRef` to refer to a window expression
{{{#!sql
SELECT "expressions_window_employee"."id",
"expressions_window_employee"."name",
"expressions_window_employee"."salary",
"expressions_window_employee"."department",
"expressions_window_employee"."hire_date",
"expressions_window_employee"."age",
"expressions_window_employee"."classification_id",
"expressions_window_employee"."bonus",
RANK() OVER (PARTITION BY
"expressions_window_employee"."department"
ORDER BY "expressions_window_employee"."salary" DESC)
AS "department_salary_rank",
(SELECT "col1"
FROM
(SELECT *
FROM
(SELECT U0."name" AS "col1",
RANK() OVER (PARTITION BY
"expressions_window_employee"."department"
ORDER BY
"expressions_window_employee"."salary" DESC) AS "qual0",
U0."age" AS "qual1"
FROM "expressions_window_employee" U0) "qualify"
WHERE "qual1" = ("qual0") ) "qualify_mask"
LIMIT 1) AS "foo"
FROM "expressions_window_employee";
}}}
The `RANK()` window within the subquery will be against the subquery table
and not the outer one which only happens to work in this case because it
happens to be the same table.
I believe the proper SQL here would be
{{{#!sql
SELECT
*,
(SELECT U0."name"
FROM "expressions_window_employee" U0
WHERE U0."age" = "department_salary_rank"
LIMIT 1) AS "foo"
FROM (
SELECT "expressions_window_employee"."id",
"expressions_window_employee"."name",
"expressions_window_employee"."salary",
"expressions_window_employee"."department",
"expressions_window_employee"."hire_date",
"expressions_window_employee"."age",
"expressions_window_employee"."classification_id",
"expressions_window_employee"."bonus",
RANK() OVER (PARTITION BY
"expressions_window_employee"."department"
ORDER BY
"expressions_window_employee"."salary" DESC) AS "department_salary_rank"
FROM "expressions_window_employee"
) subquery
}}}
Things get even more complex when filtering against a subquery annotation
that refers to a window function as that filter not requires `QUALIFY`
emulation. Things get even more complex when using nested subqueries
referring to an outerquery window function.
I would strongly suggest we opt for explicitly not supporting subqueries
references to outerref window function instead of trying to get 4.2
working with them and create a new feature request to add support for them
as they require special care to get working properly.
{{{#!patch
diff --git a/django/db/models/expressions.py
b/django/db/models/expressions.py
index 2d8a5c5919..a426195b0d 100644
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -857,6 +857,10 @@ def as_sql(self, *args, **kwargs):
def resolve_expression(self, *args, **kwargs):
col = super().resolve_expression(*args, **kwargs)
+ if col.contains_over_clause:
+ raise NotSupportedError(
+ f"Referencing outer query window expression is not
supported: {self.name}"
+ )
# FIXME: Rename possibly_multivalued to multivalued and fix
detection
# for non-multivalued JOINs (e.g. foreign key fields). This
should take
# into account only many-to-many and one-to-many relationships.
diff --git a/django/db/models/sql/compiler.py
b/django/db/models/sql/compiler.py
index 6929f216b4..21234fb6b0 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -674,7 +674,7 @@ def collect_replacements(expressions):
)
)
inner_query_compiler = inner_query.get_compiler(
- self.using, elide_empty=self.elide_empty
+ self.using, connection=self.connection,
elide_empty=self.elide_empty
)
inner_sql, inner_params = inner_query_compiler.as_sql(
# The limits must be applied to the outer query to avoid
pruning
diff --git a/tests/expressions_window/tests.py
b/tests/expressions_window/tests.py
index 027fc9c25c..a27c4f222f 100644
--- a/tests/expressions_window/tests.py
+++ b/tests/expressions_window/tests.py
@@ -1578,7 +1578,7 @@ def test_invalid_filter(self):
list(qs.exclude(window=1, department="Accounting"))
-class WindowUnsupportedTests(TestCase):
+class WindowUnsupportedTests(SimpleTestCase):
def test_unsupported_backend(self):
msg = "This backend does not support window expressions."
with mock.patch.object(connection.features,
"supports_over_clause", False):
@@ -1587,6 +1587,22 @@ def test_unsupported_backend(self):
dense_rank=Window(expression=DenseRank())
).get()
+ def test_filter_subquery(self):
+ qs = Employee.objects.annotate(
+ department_salary_rank=Window(
+ Rank(), partition_by="department", order_by="-salary"
+ )
+ )
+ msg = "Referencing outer query window expression is not
supported: department_salary_rank"
+ with self.assertRaisesMessage(NotSupportedError, msg):
+ qs.annotate(
+ employee_name=Subquery(
+ Employee.objects.filter(
+ age=OuterRef("department_salary_rank")
+ ).values("name")[:1]
+ )
+ )
+
class NonQueryWindowTests(SimpleTestCase):
def test_window_repr(self):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: window orm filter | Triage Stage: Accepted
subquery |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Jannis, let's follow Simon's recommendation.
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: window orm filter | Triage Stage: Accepted
subquery |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jannis Vajen):
Dear Mariusz, dear Simon,
I updated the old pull request I opened before with a new commit
containing Simon's suggestions.
[https://github.com/django/django/pull/16597 Please have a look].
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: window orm filter | Triage Stage: Accepted
subquery |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
Thanks Jannis! To make it clear, I think we should definitely have a
ticket to add outer reference to window support but it seems impossible to
do so before the 4.2 release.
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: window orm filter | Triage Stage: Accepted
subquery |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:6 Simon Charette]:
> Thanks Jannis! To make it clear, I think we should definitely have a
ticket to add outer reference to window support but it seems impossible to
do so before the 4.2 release.
A new ticket is probably not required as
[https://code.djangoproject.com/ticket/28333#comment:34 #28333 is still
open].
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution:
subquery | checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Component: Database layer | Version: 4.2
(models, ORM) |
Keywords: window orm filter | Triage Stage: Ready for
subquery | checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"c67ea79aa981ae82595d89f8018a41fcd842e7c9" c67ea79a]:
{{{
#!CommitTicketReference repository=""
revision="c67ea79aa981ae82595d89f8018a41fcd842e7c9"
Fixed #34368 -- Made subquery raise NotSupportedError when referencing
outer window expression.
Regression in f387d024fc75569d2a4a338bfda76cc2f328f627.
Co-authored-by: Jannis Vajen <jva...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Jannis Vajen | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: window orm filter | Triage Stage: Ready for
subquery | checkin
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"fc15d11f2eb26fe3d5c946e69223880bfe53e92b" fc15d11f]:
{{{
#!CommitTicketReference repository=""
revision="fc15d11f2eb26fe3d5c946e69223880bfe53e92b"
[4.2.x] Fixed #34368 -- Made subquery raise NotSupportedError when
referencing outer window expression.
Regression in f387d024fc75569d2a4a338bfda76cc2f328f627.
Co-authored-by: Jannis Vajen <jva...@gmail.com>
Backport of c67ea79aa981ae82595d89f8018a41fcd842e7c9 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34368#comment:10>