From 21f8be76d43aa1ee5ae41c1e0a428cfea1f231c1 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Wed, 2 Apr 2025 18:53:36 -0400 Subject: [PATCH] Fixed #36288 -- Addressed improper handling of duplicates in values_list(). Now that selected aliases are stored in sql.Query.selected: dict[str, Any] the values_list() method must ensures that duplicate field name references are assigned unique aliases. Refs #28900. Regression in 65ad4ade74dc9208b9d686a451cd6045df0c9c3a. Thanks Claude for the report. --- django/db/models/query.py | 29 +++++++++++++++++++---------- docs/releases/5.2.1.txt | 4 ++++ tests/composite_pk/test_values.py | 12 ++++++------ tests/queries/tests.py | 6 ++++++ 4 files changed, 35 insertions(+), 16 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 1da0944569..e019dd6db9 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -1371,24 +1371,33 @@ class QuerySet(AltersData): "field." ) - field_names = {f for f in fields if not hasattr(f, "resolve_expression")} + field_names = {f: False for f in fields if not hasattr(f, "resolve_expression")} _fields = [] expressions = {} counter = 1 for field in fields: + field_name = field + expression = None if hasattr(field, "resolve_expression"): - field_id_prefix = getattr( + field_name = getattr( field, "default_alias", field.__class__.__name__.lower() ) - while True: - field_id = field_id_prefix + str(counter) + expression = field + # For backward compatibility reasons expressions are always + # prefixed with the counter even if their default alias doesn't + # collide with field names. Changing this logic could break + # some usage of named=True. + seen = True + elif seen := field_names[field_name]: + expression = F(field_name) + if seen: + field_name_prefix = field_name + while (field_name := f"{field_name_prefix}{counter}") in field_names: counter += 1 - if field_id not in field_names: - break - expressions[field_id] = field - _fields.append(field_id) - else: - _fields.append(field) + if expression is not None: + expressions[field_name] = expression + field_names[field_name] = True + _fields.append(field_name) clone = self._values(*_fields, **expressions) clone._iterable_class = ( diff --git a/docs/releases/5.2.1.txt b/docs/releases/5.2.1.txt index 139ce32da1..cc12e7b34f 100644 --- a/docs/releases/5.2.1.txt +++ b/docs/releases/5.2.1.txt @@ -32,3 +32,7 @@ Bugfixes * Fixed a regression in Django 5.2 that caused a crash when using ``QuerySet.select_for_update(of=(…))`` with ``values()/values_list()`` including expressions (:ticket:`36301`). + +* Fixed a regression in Django 5.2 that caused improper values to be returned + from ``QuerySet.values_list()`` when duplicate field names were specified + (:ticket:`36288`). diff --git a/tests/composite_pk/test_values.py b/tests/composite_pk/test_values.py index a3c7a589cc..03a9a85496 100644 --- a/tests/composite_pk/test_values.py +++ b/tests/composite_pk/test_values.py @@ -128,18 +128,18 @@ class CompositePKValuesTests(TestCase): self.assertSequenceEqual( User.objects.values_list("pk", "pk").order_by("pk"), ( - (self.user_1.pk,), - (self.user_2.pk,), - (self.user_3.pk,), + (self.user_1.pk, self.user_1.pk), + (self.user_2.pk, self.user_2.pk), + (self.user_3.pk, self.user_3.pk), ), ) with self.subTest('User.objects.values_list("pk", "id", "pk", "id")'): self.assertSequenceEqual( User.objects.values_list("pk", "id", "pk", "id").order_by("pk"), ( - (self.user_1.pk, self.user_1.id), - (self.user_2.pk, self.user_2.id), - (self.user_3.pk, self.user_3.id), + (self.user_1.pk, self.user_1.id, self.user_1.pk, self.user_1.id), + (self.user_2.pk, self.user_2.id, self.user_2.pk, self.user_2.id), + (self.user_3.pk, self.user_3.id, self.user_3.pk, self.user_3.id), ), ) diff --git a/tests/queries/tests.py b/tests/queries/tests.py index c429a93af3..38b0a5ddfa 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2668,6 +2668,12 @@ class ValuesQuerysetTests(TestCase): qs = qs.values_list("num", flat=True) self.assertSequenceEqual(qs, [72]) + def test_duplicate_values_list(self): + value = Number.objects.values_list("num", "num").get() + self.assertEqual(value, (72, 72)) + value = Number.objects.values_list(F("num"), F("num")).get() + self.assertEqual(value, (72, 72)) + def test_extra_values(self): # testing for ticket 14930 issues qs = Number.objects.extra(