Skip to content

Conversation

JaeHyuckSa
Copy link
Contributor

Trac ticket number

ticket-36431

Branch description

Thanks to @jacobtylerwalls and @charettes for the tests.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

@JaeHyuckSa JaeHyuckSa changed the title Fixed #36431 -- Returned tuples for multi-column ForeignObject in values()/values_list(). Fixed #36431 -- Returned tuples for multi-column ForeignObject in values()/values_list(). Aug 24, 2025
Copy link
Member

@charettes charettes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @JaeHyuckSa

Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @JaeHyuckSa

I made this a release blocker for 5.2, could you rebase main and add a release note to 5.2.6. Something like

* Fixed a bug where using ``QuerySet.values()`` or ``values_list()`` with a
  ``ForeignObject`` composed of multiple fields returned incorrect results
  instead of tuples of the referenced fields (:ticket:`36431`).

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Aug 29, 2025

In good news, this PR passes this test case if I comment out the ValueError raising the message "The QuerySet value for the 'in' lookup must have 2 selected fields..." given that "user" is 1 field but returns a proper tuple:

diff --git a/tests/composite_pk/test_filter.py b/tests/composite_pk/test_filter.py
index d7ecfbec11..c4b393d6ea 100644
--- a/tests/composite_pk/test_filter.py
+++ b/tests/composite_pk/test_filter.py
@@ -460,6 +460,11 @@ class CompositePKFilterTests(TestCase):
         queryset = User.objects.filter(comments__in=subquery)
         self.assertSequenceEqual(queryset, (self.user_2,))
 
+    def test_filter_comments_by_users_subquery(self):
+        subquery = Comment.objects.filter(id=3).values("user")
+        queryset = Comment.objects.filter(user__in=subquery)
+        self.assertSequenceEqual(queryset, (self.comment_3,))
+
     def test_cannot_cast_pk(self):
         msg = "Cast expression does not support composite primary keys."
         with self.assertRaisesMessage(ValueError, msg):

Opened ticket-36584 for this.

@JaeHyuckSa
Copy link
Contributor Author

JaeHyuckSa commented Aug 29, 2025

In good news, this PR passes this test case if I comment out the ValueError raising the message "The QuerySet value for the 'in' lookup must have 2 selected fields..." given that "user" is 1 field but returns a proper tuple:

diff --git a/tests/composite_pk/test_filter.py b/tests/composite_pk/test_filter.py
index d7ecfbec11..c4b393d6ea 100644
--- a/tests/composite_pk/test_filter.py
+++ b/tests/composite_pk/test_filter.py
@@ -460,6 +460,11 @@ class CompositePKFilterTests(TestCase):
         queryset = User.objects.filter(comments__in=subquery)
         self.assertSequenceEqual(queryset, (self.user_2,))
 
+    def test_filter_comments_by_users_subquery(self):
+        subquery = Comment.objects.filter(id=3).values("user")
+        queryset = Comment.objects.filter(user__in=subquery)
+        self.assertSequenceEqual(queryset, (self.comment_3,))
+
     def test_cannot_cast_pk(self):
         msg = "Cast expression does not support composite primary keys."
         with self.assertRaisesMessage(ValueError, msg):

Opened ticket-36584 for this.

@jacobtylerwalls Interesting — I’ll pick this up as a follow-up to the current PR. Thank you very much for flagging it.

…ues()/values_list().

Thanks Jacob Walls and Simon Charette for tests.

Signed-off-by: SaJH <wogur981208@gmail.com>
Copy link
Member

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@jacobtylerwalls jacobtylerwalls merged commit bb7a770 into django:main Aug 29, 2025
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants