From ed6788bbd777ffeb1e924f1a1033d2d17e1d34d0 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Mon, 2 Sep 2024 23:51:49 -0400 Subject: [PATCH] Demonstrate how implicit ordering of extra(select) can be preserved. --- django/db/models/query.py | 41 ++++++++++++++++++++++++++++---- django/db/models/sql/compiler.py | 15 +++++++++++- tests/extra_regress/tests.py | 4 ++-- tests/queries/tests.py | 2 +- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/django/db/models/query.py b/django/db/models/query.py index 159c17d305..3d954fdf5c 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -210,9 +210,19 @@ class ValuesIterable(BaseIterable): if query.selected: names = list(query.selected) else: + # RemovedInDjango60Warning: place extra(select) entries at the + # beginning of the SELECT clause until it's completely removed. + extra_names = [] + annotation_select = [] + for alias, expression in query.annotation_select.items(): + if isinstance(expression, _ExtraSelectFirstRawSQL): + extra_names.append(alias) + else: + annotation_select.append(alias) names = [ + *extra_names, *query.values_select, - *query.annotation_select, + *annotation_select, ] indexes = range(len(names)) for row in compiler.results_iter( @@ -250,9 +260,19 @@ class NamedValuesListIterable(ValuesListIterable): names = queryset._fields else: query = queryset.query + # RemovedInDjango60Warning: place extra(select) entries at the + # beginning of the SELECT clause until it's completely removed. + extra_names = [] + annotation_select = [] + for alias, expression in query.annotation_select.items(): + if isinstance(expression, _ExtraSelectFirstRawSQL): + extra_names.append(alias) + else: + annotation_select.append(alias) names = [ + *extra_names, *query.values_select, - *query.annotation_select, + *annotation_select, ] tuple_class = create_namedtuple_class(*names) new = tuple.__new__ @@ -275,6 +295,17 @@ class FlatValuesListIterable(BaseIterable): yield row[0] +# RemovedInDjango60Warning +class _ExtraSelectFirstRawSQL(RawSQL): + """ + Internal RawSQL subclass used during the deprecation of extra(select) to + preserve the undocumented implicit ordering of members at the begining of + the SELECT clause. + """ + + implicitly_select_first = True + + class QuerySet(AltersData): """Represent a lazy database lookup for a set of objects.""" @@ -1724,8 +1755,6 @@ class QuerySet(AltersData): category=RemovedInDjango60Warning, stacklevel=2, ) - # XXX: This is mising the logic to always place extras at the begining - # of the select clause. if select_params: param_iter = iter(select_params) else: @@ -1739,7 +1768,9 @@ class QuerySet(AltersData): if pos == 0 or entry[pos - 1] != "%": entry_params.append(next(param_iter)) pos = entry.find("%s", pos + 2) - clone.query.add_annotation(RawSQL(entry, entry_params), name) + clone.query.add_annotation( + _ExtraSelectFirstRawSQL(entry, entry_params), name + ) if where: warnings.warn( "extra(where) usage is deprecated, use filter() with RawSQL instead.", diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 91e941837a..5c3166748d 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -262,9 +262,22 @@ class SQLCompiler: } selected = [] if self.query.selected is None: + # RemovedInDjango60Warning: place extra(select) entries at the + # beginning of the SELECT clause until it's completely removed. + leading_annotation_select = [] + annotation_select = [] + if self.query.values_select_all: + for alias, expression in self.query.annotation_select.items(): + if getattr(expression, "implicitly_select_first", False): + leading_annotation_select.append((alias, expression)) + else: + annotation_select.append((alias, expression)) + else: + annotation_select = self.query.annotation_select.items() selected = [ + *leading_annotation_select, *((None, col) for col in cols), - *self.query.annotation_select.items(), + *annotation_select, ] else: for alias, expression in self.query.selected.items(): diff --git a/tests/extra_regress/tests.py b/tests/extra_regress/tests.py index 55075a82bb..095e4a76cb 100644 --- a/tests/extra_regress/tests.py +++ b/tests/extra_regress/tests.py @@ -286,7 +286,7 @@ class ExtraRegressTests(TestCase): select={"foo": "first", "bar": "second", "whiz": "third"} ).values_list() ), - [(obj.pk, "first", "second", "third", "first", "second", "third")], + [("first", "second", "third", obj.pk, "first", "second", "third")], ) # Extra columns after an empty values_list() are still included @@ -296,7 +296,7 @@ class ExtraRegressTests(TestCase): select={"foo": "first", "bar": "second", "whiz": "third"} ) ), - [(obj.pk, "first", "second", "third", "first", "second", "third")], + [("first", "second", "third", obj.pk, "first", "second", "third")], ) # Extra columns ignored completely if not mentioned in values_list() diff --git a/tests/queries/tests.py b/tests/queries/tests.py index 57b8b1b22a..46d6539974 100644 --- a/tests/queries/tests.py +++ b/tests/queries/tests.py @@ -2841,7 +2841,7 @@ class ValuesQuerysetTests(TestCase): self.assertEqual(type(values).__name__, "Row") self.assertEqual( values._fields, - ("id", "num", "other_num", "another_num", "num2", "id__count"), + ("num2", "id", "num", "other_num", "another_num", "id__count"), ) self.assertEqual(values.num, 72) self.assertEqual(values.num2, 73)