[Django] #36181: Composite primary key fields cannot use __in lookup with explicit Subquery

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 5.2 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
In a recent PR, we
[https://github.com/django/django/pull/19108#discussion_r1932339414
surfaced] some inconsistencies with the resolution of explicit
`Subquery()` such that some lookups might fail:

{{{#!diff
diff --git a/tests/composite_pk/test_filter.py
b/tests/composite_pk/test_filter.py
index 937dd86652..60d43f4a52 100644
--- a/tests/composite_pk/test_filter.py
+++ b/tests/composite_pk/test_filter.py
@@ -437,6 +437,11 @@ class CompositePKFilterTests(TestCase):
queryset = User.objects.filter(comments__in=subquery)
self.assertSequenceEqual(queryset, (self.user_2,))

+ def test_explicit_subquery(self):
+ subquery = Subquery(User.objects.values("pk"))
+ self.assertEqual(User.objects.filter(pk__in=subquery).count(), 5)
+
self.assertEqual(Comment.objects.filter(user__in=subquery).count(), 5)
+
def test_cannot_cast_pk(self):
msg = "Cast expression does not support composite primary keys."
with self.assertRaisesMessage(ValueError, msg):
}}}

gives
{{{#!py
======================================================================
ERROR: test_explicit_subquery
(composite_pk.test_filter.CompositePKFilterTests.test_explicit_subquery)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/.../django/tests/composite_pk/test_filter.py", line 418, in
test_explicit_subquery
self.assertEqual(Comment.objects.filter(user__in=subquery).count(), 5)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/query.py", line 603, in count
return self.query.get_count(using=self.db)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/sql/query.py", line 644, in
get_count
return obj.get_aggregation(using, {"__count": Count("*")})["__count"]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/sql/query.py", line 626, in
get_aggregation
result = compiler.execute_sql(SINGLE)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/sql/compiler.py", line 1610, in
execute_sql
sql, params = self.as_sql()
^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/sql/compiler.py", line 794, in
as_sql
self.compile(self.where) if self.where is not None else ("", [])
^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/sql/compiler.py", line 577, in
compile
sql, params = node.as_sql(self, self.connection)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/sql/where.py", line 151, in
as_sql
sql, params = compiler.compile(child)
^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/sql/compiler.py", line 577, in
compile
sql, params = node.as_sql(self, self.connection)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/fields/related_lookups.py",
line 86, in as_sql
SubqueryConstraint(
File "/Users/.../django/django/db/models/sql/where.py", line 358, in
__init__
query_object.clear_ordering(clear_default=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Subquery' object has no attribute 'clear_ordering'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36181>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* severity: Normal => Release blocker
* stage: Unreviewed => Accepted

Comment:

Well good news, I think the work on #36149 managed to make
`SubqueryConstraint` completely irrelevant while solving this issue.

{{{#!diff
diff --git a/django/db/backends/mysql/compiler.py
b/django/db/backends/mysql/compiler.py
index 2ec6bea2f1..0291b76c70 100644
--- a/django/db/backends/mysql/compiler.py
+++ b/django/db/backends/mysql/compiler.py
@@ -1,28 +1,20 @@
from django.core.exceptions import FieldError, FullResultSet
from django.db.models.expressions import Col
-from django.db.models.sql import compiler
+from django.db.models.sql.compiler import SQLAggregateCompiler,
SQLCompiler
+from django.db.models.sql.compiler import SQLDeleteCompiler as
BaseSQLDeleteCompiler
+from django.db.models.sql.compiler import SQLInsertCompiler
+from django.db.models.sql.compiler import SQLUpdateCompiler as
BaseSQLUpdateCompiler

+__all__ = [
+ "SQLAggregateCompiler",
+ "SQLCompiler",
+ "SQLDeleteCompiler",
+ "SQLInsertCompiler",
+ "SQLUpdateCompiler",
+]

-class SQLCompiler(compiler.SQLCompiler):
- def as_subquery_condition(self, alias, columns, compiler):
- qn = compiler.quote_name_unless_alias
- qn2 = self.connection.ops.quote_name
- sql, params = self.as_sql()
- return (
- "(%s) IN (%s)"
- % (
- ", ".join("%s.%s" % (qn(alias), qn2(column)) for column
in columns),
- sql,
- ),
- params,
- )
-
-
-class SQLInsertCompiler(compiler.SQLInsertCompiler, SQLCompiler):
- pass

-
-class SQLDeleteCompiler(compiler.SQLDeleteCompiler, SQLCompiler):
+class SQLDeleteCompiler(BaseSQLDeleteCompiler):
def as_sql(self):
# Prefer the non-standard DELETE FROM syntax over the SQL
generated by
# the SQLDeleteCompiler's default implementation when multiple
tables
@@ -52,7 +44,7 @@ def as_sql(self):
return " ".join(result), tuple(params)


-class SQLUpdateCompiler(compiler.SQLUpdateCompiler, SQLCompiler):
+class SQLUpdateCompiler(BaseSQLUpdateCompiler):
def as_sql(self):
update_query, update_params = super().as_sql()
# MySQL and MariaDB support UPDATE ... ORDER BY syntax.
@@ -78,7 +70,3 @@ def as_sql(self):
# removed in .update() and cannot be resolved.
pass
return update_query, update_params
-
-
-class SQLAggregateCompiler(compiler.SQLAggregateCompiler, SQLCompiler):
- pass
diff --git a/django/db/models/fields/related_lookups.py
b/django/db/models/fields/related_lookups.py
index 38d6308f53..9fc7db7c34 100644
--- a/django/db/models/fields/related_lookups.py
+++ b/django/db/models/fields/related_lookups.py
@@ -84,21 +84,12 @@ def get_prep_lookup(self):

def as_sql(self, compiler, connection):
if isinstance(self.lhs, ColPairs):
- from django.db.models.sql.where import SubqueryConstraint
-
if self.rhs_is_direct_value():
values = [get_normalized_value(value, self.lhs) for value
in self.rhs]
lookup = TupleIn(self.lhs, values)
- return compiler.compile(lookup)
else:
- return compiler.compile(
- SubqueryConstraint(
- self.lhs.alias,
- [target.column for target in self.lhs.targets],
- [source.name for source in self.lhs.sources],
- self.rhs,
- ),
- )
+ lookup = TupleIn(self.lhs, self.rhs)
+ return compiler.compile(lookup)

return super().as_sql(compiler, connection)

diff --git a/django/db/models/fields/tuple_lookups.py
b/django/db/models/fields/tuple_lookups.py
index 98959a6161..e701c7d3de 100644
--- a/django/db/models/fields/tuple_lookups.py
+++ b/django/db/models/fields/tuple_lookups.py
@@ -2,7 +2,13 @@

from django.core.exceptions import EmptyResultSet
from django.db.models import Field
-from django.db.models.expressions import ColPairs, Func,
ResolvedOuterRef, Value
+from django.db.models.expressions import (
+ ColPairs,
+ Func,
+ ResolvedOuterRef,
+ Subquery,
+ Value,
+)
from django.db.models.lookups import (
Exact,
GreaterThan,
@@ -301,7 +307,7 @@ def check_rhs_elements_length_equals_lhs_length(self):
)

def check_rhs_is_query(self):
- if not isinstance(self.rhs, Query):
+ if not isinstance(self.rhs, (Query, Subquery)):
lhs_str = self.get_lhs_str()
rhs_cls = self.rhs.__class__.__name__
raise ValueError(
diff --git a/django/db/models/sql/compiler.py
b/django/db/models/sql/compiler.py
index 3bfb3bd631..ef10f00f20 100644
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -1661,19 +1661,6 @@ def execute_sql(
return list(result)
return result

- def as_subquery_condition(self, alias, columns, compiler):
- qn = compiler.quote_name_unless_alias
- qn2 = self.connection.ops.quote_name
- query = self.query.clone()
-
- for index, select_col in enumerate(query.select):
- lhs_sql, lhs_params = self.compile(select_col)
- rhs = "%s.%s" % (qn(alias), qn2(columns[index]))
- query.where.add(RawSQL("%s = %s" % (lhs_sql, rhs),
lhs_params), AND)
-
- sql, params = query.as_sql(compiler, self.connection)
- return "EXISTS %s" % sql, params
-
def explain_query(self):
result = list(self.execute_sql())
# Some backends return 1 item tuples with strings, and others
return
diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py
index 0fded5cce3..82f96aa6ec 100644
--- a/django/db/models/sql/where.py
+++ b/django/db/models/sql/where.py
@@ -343,23 +343,3 @@ def __init__(self, sqls, params):
def as_sql(self, compiler=None, connection=None):
sqls = ["(%s)" % sql for sql in self.sqls]
return " AND ".join(sqls), list(self.params or ())
-
-
-class SubqueryConstraint:
- # Even if aggregates or windows would be used in a subquery,
- # the outer query isn't interested about those.
- contains_aggregate = False
- contains_over_clause = False
-
- def __init__(self, alias, columns, targets, query_object):
- self.alias = alias
- self.columns = columns
- self.targets = targets
- query_object.clear_ordering(clear_default=True)
- self.query_object = query_object
-
- def as_sql(self, compiler, connection):
- query = self.query_object
- query.set_values(self.targets)
- query_compiler = query.get_compiler(connection=connection)
- return query_compiler.as_subquery_condition(self.alias,
self.columns, compiler)
diff --git a/tests/composite_pk/test_filter.py
b/tests/composite_pk/test_filter.py
index 937dd86652..4b753fb396 100644
--- a/tests/composite_pk/test_filter.py
+++ b/tests/composite_pk/test_filter.py
@@ -442,6 +442,11 @@ def test_cannot_cast_pk(self):
with self.assertRaisesMessage(ValueError, msg):
Comment.objects.filter(text__gt=Cast(F("pk"),
TextField())).count()

+ def test_explicit_subquery(self):
+ subquery = Subquery(User.objects.values("pk"))

+ self.assertEqual(User.objects.filter(pk__in=subquery).count(), 4)
+
self.assertEqual(Comment.objects.filter(user__in=subquery).count(), 5)
+
def test_filter_case_when(self):
msg = "When expression does not support composite primary keys."
with self.assertRaisesMessage(ValueError, msg):
}}}

Getting rid of `SuqueryConstraint` and `as_subquery_condition` was a goal
of #373 so that's a great win!
--
Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:1>

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Simon
| Charette
Type: Bug | Status: assigned

Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* has_patch: 0 => 1
* owner: (none) => Simon Charette
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:2>

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:3>

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Simon
| Charette

Type: Bug | Status: closed

Component: Database layer | Version: 5.2
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"8561100425876bde3be4b2a22324655f74ff9609" 8561100]:
{{{#!CommitTicketReference repository=""
revision="8561100425876bde3be4b2a22324655f74ff9609"
Fixed #36181 -- Allowed Subquery usage in __in lookups against composite
pks.

Thanks Jacob Walls for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:4>

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"d386405e04dac50656af50d100a14efdf8c58e8f" d386405e]:
{{{#!CommitTicketReference repository=""
revision="d386405e04dac50656af50d100a14efdf8c58e8f"
Refs #36181 -- Removed the obsolete SubqueryConstraint machinery.

Adding proper support for subquery right-hand-sides to TupleIn made it
obsolete.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:5>

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"771c250b106f8f4beda42c104c60498ed568b5e2" 771c250]:
{{{#!CommitTicketReference repository=""
revision="771c250b106f8f4beda42c104c60498ed568b5e2"
[5.2.x] Fixed #36181 -- Allowed Subquery usage in __in lookups against

composite pks.

Thanks Jacob Walls for the report.

Backport of 8561100425876bde3be4b2a22324655f74ff9609 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:6>

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by GitHub <noreply@…>):

In [changeset:"c80c81163e0d31aefc82eafef2b4807cde6ab0b5" c80c8116]:
{{{#!CommitTicketReference repository=""
revision="c80c81163e0d31aefc82eafef2b4807cde6ab0b5"
[5.2.x] Refs #36181 -- Removed the obsolete SubqueryConstraint machinery.

Adding proper support for subquery right-hand-sides to TupleIn made it
obsolete.

Backport of d386405e04dac50656af50d100a14efdf8c58e8f from main

Co-authored-by: Simon Charette <chare...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:7>

[Django]

#36181: Composite primary key fields cannot use __in lookup with explicit Subquery
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Simon
| Charette
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"fd569dd45bf0746378faf7f65172497f21ed27f0" fd569dd]:
{{{#!CommitTicketReference repository=""
revision="fd569dd45bf0746378faf7f65172497f21ed27f0"
Fixed #36210, Refs #36181 -- Allowed Subquery usage in further lookups
against composite pks.

Follow-up to 8561100425876bde3be4b2a22324655f74ff9609.

Co-authored-by: Simon Charette <chare...@gmail.com>
}}}
--

Ticket URL: <https://code.djangoproject.com/ticket/36181#comment:8>