Skip to content

Commit 0c17b35

Browse files
Collapse repeated prefetch lookups on the optimizer path (v0.19.1).
An interface query carries a sibling fragment per implementing type, so the same field is walked more than once and each pass builds its own queryset. Django compares prefetch querysets by identity and rejects a repeated target, which surfaced as "'x' lookup was already seen with a different queryset". The collapse existed but only ran when loading relations onto rows already in hand, not when building the query. It went unnoticed because duplicates were previously bare paths, which Django tolerates; on= made them carry a queryset, which it does not. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 5830ef4 commit 0c17b35

6 files changed

Lines changed: 199 additions & 5 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "strawberry-orm"
3-
version = "0.19.0"
3+
version = "0.19.1"
44
description = "Unified, backend-agnostic ORM abstraction for Strawberry GraphQL"
55
readme = "README.md"
66
license = "MIT"

src/strawberry_orm/backends/django.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,10 +1048,18 @@ def optimize() -> Any:
10481048
select_related, prefetch_related = self._relation_lookups(
10491049
store, model, info
10501050
)
1051+
# One relation can be reached twice - two aliases, or the same
1052+
# field under sibling fragments of an interface - and each pass
1053+
# builds its own queryset. Django compares those by identity, so
1054+
# duplicates have to be collapsed before they reach it.
10511055
if select_related:
1052-
optimized_query = optimized_query.select_related(*select_related)
1056+
optimized_query = optimized_query.select_related(
1057+
*_dedupe_lookups(select_related)
1058+
)
10531059
if prefetch_related:
1054-
optimized_query = optimized_query.prefetch_related(*prefetch_related)
1060+
optimized_query = optimized_query.prefetch_related(
1061+
*_dedupe_lookups(prefetch_related)
1062+
)
10551063

10561064
return list(optimized_query)
10571065

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
"""One relation reached twice must not collide in the prefetch.
2+
3+
An interface query carries a sibling fragment per implementing type, so the
4+
same field is walked more than once and each pass builds its own queryset.
5+
Django compares prefetch querysets by identity and rejects a repeated target,
6+
which surfaced as ``'x' lookup was already seen with a different queryset``.
7+
"""
8+
9+
import pytest
10+
import strawberry
11+
12+
from strawberry_orm import StrawberryORM
13+
from strawberry_orm.types import auto
14+
15+
16+
def _orm():
17+
return StrawberryORM.for_django(warn_missing_scope=False, lazy_resolution="off")
18+
19+
20+
@pytest.mark.django_db
21+
class TestDuplicateLookups:
22+
def test_the_same_on_field_under_two_fragments(self, seed, User, Post):
23+
orm = _orm()
24+
25+
@orm.type(Post)
26+
class PostType:
27+
id: auto
28+
title: auto
29+
30+
@orm.type(User)
31+
class UserType:
32+
id: auto
33+
name: auto
34+
published: list[PostType] = orm.field.eager(
35+
on="posts", scope=lambda qs, info: qs.filter(is_published=True)
36+
)
37+
38+
@strawberry.type
39+
class Query:
40+
users: list[UserType] = orm.field.eager()
41+
42+
# The field is named twice in one selection, which is what an
43+
# interface's sibling fragments produce.
44+
result = orm.schema(query=Query).execute_sync(
45+
"""
46+
{
47+
users {
48+
published { title }
49+
... on UserType { published { title } }
50+
}
51+
}
52+
""",
53+
context_value={},
54+
)
55+
assert result.errors is None, result.errors
56+
titles = {p["title"] for u in result.data["users"] for p in u["published"]}
57+
assert "Draft Post" not in titles
58+
59+
def test_an_on_field_also_named_in_using(self, seed, User, Post):
60+
"""The optimizer reaches it twice: once selected, once as a hint."""
61+
orm = _orm()
62+
63+
@orm.type(Post)
64+
class PostType:
65+
id: auto
66+
title: auto
67+
68+
@orm.type(User)
69+
class UserType:
70+
id: auto
71+
name: auto
72+
published: list[PostType] = orm.field.eager(
73+
on="posts", scope=lambda qs, info: qs.filter(is_published=True)
74+
)
75+
76+
@orm.field.lazy(using=["posts"])
77+
def post_count(self, info: strawberry.Info) -> int:
78+
return len(list(self.posts.all()))
79+
80+
@strawberry.type
81+
class Query:
82+
users: list[UserType] = orm.field.eager()
83+
84+
result = orm.schema(query=Query).execute_sync(
85+
"{ users { published { title } postCount } }", context_value={}
86+
)
87+
assert result.errors is None, result.errors
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
"""One relation reached twice must not collide in the prefetch.
2+
3+
An interface query carries a sibling fragment per implementing type, so the
4+
same field is walked more than once and each pass builds its own queryset.
5+
Django compares prefetch querysets by identity and rejects a repeated target,
6+
which surfaced as ``'x' lookup was already seen with a different queryset``.
7+
"""
8+
9+
import pytest
10+
import strawberry
11+
12+
from strawberry_orm import StrawberryORM
13+
from strawberry_orm.types import auto
14+
from tests.backends.sqlalchemy.models import Post as SAPost
15+
from tests.backends.sqlalchemy.models import User as SAUser
16+
17+
18+
def _orm():
19+
return StrawberryORM.for_sqlalchemy(
20+
dialect="sqlite", warn_missing_scope=False, lazy_resolution="off"
21+
)
22+
23+
24+
class TestDuplicateLookups:
25+
@pytest.fixture(autouse=True)
26+
def _session(self, sa_session, seed):
27+
self._s = sa_session
28+
29+
def test_the_same_on_field_under_two_fragments(self):
30+
orm = _orm()
31+
32+
@orm.type(SAPost)
33+
class PostType:
34+
id: auto
35+
title: auto
36+
37+
@orm.type(SAUser)
38+
class UserType:
39+
id: auto
40+
name: auto
41+
published: list[PostType] = orm.field.eager(
42+
on="posts",
43+
scope=lambda qs, info: qs.where(SAPost.is_published.is_(True)),
44+
)
45+
46+
@strawberry.type
47+
class Query:
48+
users: list[UserType] = orm.field.eager()
49+
50+
# The field is named twice in one selection, which is what an
51+
# interface's sibling fragments produce.
52+
result = orm.schema(query=Query).execute_sync(
53+
"""
54+
{
55+
users {
56+
published { title }
57+
... on UserType { published { title } }
58+
}
59+
}
60+
""",
61+
context_value={"session": self._s},
62+
)
63+
assert result.errors is None, result.errors
64+
titles = {p["title"] for u in result.data["users"] for p in u["published"]}
65+
assert "Draft Post" not in titles
66+
67+
def test_an_on_field_also_named_in_using(self):
68+
"""The optimizer reaches it twice: once selected, once as a hint."""
69+
orm = _orm()
70+
71+
@orm.type(SAPost)
72+
class PostType:
73+
id: auto
74+
title: auto
75+
76+
@orm.type(SAUser)
77+
class UserType:
78+
id: auto
79+
name: auto
80+
published: list[PostType] = orm.field.eager(
81+
on="posts",
82+
scope=lambda qs, info: qs.where(SAPost.is_published.is_(True)),
83+
)
84+
85+
@orm.field.lazy(using=["posts"])
86+
def post_count(self, info: strawberry.Info) -> int:
87+
return len(list(self.posts))
88+
89+
@strawberry.type
90+
class Query:
91+
users: list[UserType] = orm.field.eager()
92+
93+
result = orm.schema(query=Query).execute_sync(
94+
"{ users { published { title } postCount } }",
95+
context_value={"session": self._s},
96+
)
97+
assert result.errors is None, result.errors

tests/test_backend_parity.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,10 @@
9898
# that rather than the behaviour the other two share.
9999
"test_relation_connection.py",
100100
# on= is refused on Tortoise, so its test_on.py asserts that rather than
101-
# the behaviour the other two share.
101+
# the behaviour the other two share, and the duplicate-lookup case it
102+
# brings cannot arise there at all.
102103
"test_on.py",
104+
"test_on_duplicate_lookups.py",
103105
"test_query_session_resolution.py",
104106
"test_query_queryset_detection.py",
105107
"test_query_async_session.py",

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)