[Django] #36431: values() query on ForeignObject discards additional columns

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 4.2 | Severity: Normal
Keywords: composite primary | Triage Stage:
key | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
For a multi-column field using `ForeignObject` as modeled
[https://docs.djangoproject.com/en/5.2/topics/composite-primary-key
/#composite-primary-keys-and-relations here], `.values("user")` will
discard columns after the first, and `values("user", "integer")` will
select the second column of `user` into the `integer` namespace.

Here's a failing test for tests/composite_pk/test_values.py:
{{{#!py
def test_foreign_object_values(self):
Comment.objects.create(id=1, user=self.user_1, integer=42)
values = list(Comment.objects.values("user", "integer"))
self.assertEqual(values[0]["integer"], 42)
}}}
{{{
======================================================================
FAIL: test_foreign_object_values
(composite_pk.test_values.CompositePKValuesTests.test_foreign_object_values)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/.../django/tests/composite_pk/test_values.py", line 217, in
test_foreign_object_values
self.assertEqual(values[0]["integer"], 42)
~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 1 != 42
}}}

And a similar rough test for foreign_objects/tests.py that fails on Django
4.2, to remove the composite primary key from discussion. (If this model
had more fields, this values query would be more realistic):
{{{#!py
class ForeignObjectValues(TestCase):
def test_foreign_object_values(self):
from .models import Customer
customer_1 = Customer.objects.create(customer_id=1, company="a")
customer_2 = Customer.objects.create(customer_id=1, company="b")
company_a_customers =
Customer.objects.filter(company="a").values("address", "company")
self.assertSequenceEqual(
Customer.objects.exclude(company__in=[cust["company"] for cust
in company_a_customers]),
[customer_2], # Fails, all companies included
)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36431>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |

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

Changes (by Simon Charette):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:1>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* cc: Simon Charette (added)

Comment:

On interesting dilemma we'll have to figure out here is what exactly to
return when a foreign object composed of more than one `from_fields` is
selected through `values` and `values_list`.

With composite primary keys we've made usages of `values("pk")` turned
into `values(Tuple(*pk_field_attnames))` and not
`values(*pk_field_attnames)` so we should be coherent here IMO which means
that we likely want something like

{{{#!diff
diff --git a/tests/composite_pk/test_values.py
b/tests/composite_pk/test_values.py
index 03a9a85496..cc45db40bc 100644
--- a/tests/composite_pk/test_values.py
+++ b/tests/composite_pk/test_values.py
@@ -3,7 +3,7 @@

from django.test import TestCase

-from .models import Post, Tenant, User
+from .models import Comment, Post, Tenant, User


class CompositePKValuesTests(TestCase):
@@ -210,3 +210,9 @@ def test_values(self):
{"pk": self.user_3.pk, "id": self.user_3.id},
),
)
+
+ def test_foreign_object_values(self):
+ Comment.objects.create(id=1, user=self.user_1, integer=42)
+ values = list(Comment.objects.values("user", "integer"))
+ self.assertEqual(values[0]["user"], (self.user_1.tenant_id,
self.user_1.id))
+ self.assertEqual(values[0]["integer"], 42)
}}}

In other words I suggest we double down on making sure references to
composite fields (the only two we support today is composite pk and
foreign objects with multiple fields) in `values` and `values_list` return
tuples instead of flattening them as we've taken this approach for
`CompositePrimaryKey` (which is now part of the public API) and that
`ForeignObject` is still private and has been broken since its
introduction in this regard.
--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:2>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Colleen
| Dunlap
Type: Bug | Status: assigned

Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Colleen Dunlap):

* owner: (none) => Colleen Dunlap
* status: new => assigned

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

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: Bug | Status: new

Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Colleen Dunlap):

* owner: Colleen Dunlap => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:4>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:

| JaeHyuckSa
Type: Bug | Status: assigned

Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by JaeHyuckSa):

* owner: (none) => JaeHyuckSa

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:5>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |

Has patch: 1 | Needs documentation: 0

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

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:6>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Simon Charette):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:7>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette):

I suspect the logic will be able to be simplified through #35956
(`setup_join` will most likely need to return a single multi-valued target
instead of being multi-valued itself) but the proposed patch seems focused
enough to be accepted as is.
--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:8>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned
Component: Database layer | Version: 4.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: composite primary | Triage Stage: Accepted
key |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by JaeHyuckSa):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:9>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned

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

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

Changes (by Sarah Boyce):

* severity: Normal => Release blocker
* version: 4.2 => 5.2

Comment:

Marking as a release blocker for 5.2, even though the issue has existed
within `ForeignObject` for longer, since in 5.2
[https://docs.djangoproject.com/en/5.2/topics/composite-primary-key
/#composite-primary-keys-and-relations we've opted to start documenting]
`ForeignObject`'s usage with `CompositePrimaryKey` (whereas before
`ForeignObject` was not documented/public)
--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:11>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: CompositePrimaryKey | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:12>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: CompositePrimaryKey | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by JaeHyuckSa):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:13>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: CompositePrimaryKey | 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 Jacob Walls):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:14>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa

Type: Bug | Status: closed

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

Severity: Release blocker | Resolution: fixed

Keywords: CompositePrimaryKey | 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 Jacob Walls <jacobtylerwalls@…>):

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

Comment:

In [changeset:"bb7a7701b1a0e8fffe14dcebf5d5bac7f176c02a" bb7a770]:
{{{#!CommitTicketReference repository=""
revision="bb7a7701b1a0e8fffe14dcebf5d5bac7f176c02a"
Fixed #36431 -- Returned tuples for multi-column ForeignObject in
values()/values_list().

Thanks Jacob Walls and Simon Charette for tests.

Signed-off-by: SaJH <wogur...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:15>

[Django]

#36431: values() query on ForeignObject discards additional columns
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner:
| JaeHyuckSa
Type: Bug | Status: closed
Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: CompositePrimaryKey | 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 Jacob Walls <jacobtylerwalls@…>):

In [changeset:"ace59cb83b87a4fdeab29424ea134e78de24fb27" ace59cb]:
{{{#!CommitTicketReference repository=""
revision="ace59cb83b87a4fdeab29424ea134e78de24fb27"
[5.2.x] Fixed #36431 -- Returned tuples for multi-column ForeignObject in

values()/values_list().

Thanks Jacob Walls and Simon Charette for tests.

Signed-off-by: SaJH <wogur...@gmail.com>

Backport of bb7a7701b1a0e8fffe14dcebf5d5bac7f176c02a from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36431#comment:16>