diff options
| author | Simon Charette <charette.s@gmail.com> | 2023-03-28 00:13:00 -0400 |
|---|---|---|
| committer | Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> | 2024-07-03 16:36:25 +0200 |
| commit | 65ad4ade74dc9208b9d686a451cd6045df0c9c3a (patch) | |
| tree | 641c0c1e6264a3f23212ee383b46c25fd4d0f0a1 /django/db/models/sql | |
| parent | 2e47dde438d689199934bca0967152a3b0e8a95f (diff) | |
Refs #28900 -- Made SELECT respect the order specified by values(*selected).
Previously the order was always extra_fields + model_fields + annotations with
respective local ordering inferred from the insertion order of *selected.
This commits introduces a new `Query.selected` propery that keeps tracks of the
global select order as specified by on values assignment. This is crucial
feature to allow the combination of queries mixing annotations and table
references.
It also allows the removal of the re-ordering shenanigans perform by
ValuesListIterable in order to re-map the tuples returned from the database
backend to the order specified by values_list() as they'll be in the right
order at query compilation time.
Refs #28553 as the initially reported issue that was only partially fixed
for annotations by d6b6e5d0fd4e6b6d0183b4cf6e4bd4f9afc7bf67.
Thanks Mariusz Felisiak and Sarah Boyce for review.
Diffstat (limited to 'django/db/models/sql')
| -rw-r--r-- | django/db/models/sql/compiler.py | 45 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 47 |
2 files changed, 63 insertions, 29 deletions
diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 7377e555c3..d606505cdf 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -247,11 +247,6 @@ class SQLCompiler: select = [] klass_info = None annotations = {} - select_idx = 0 - for alias, (sql, params) in self.query.extra_select.items(): - annotations[alias] = select_idx - select.append((RawSQL(sql, params), alias)) - select_idx += 1 assert not (self.query.select and self.query.default_cols) select_mask = self.query.get_select_mask() if self.query.default_cols: @@ -261,19 +256,39 @@ class SQLCompiler: # any model. cols = self.query.select if cols: - select_list = [] - for col in cols: - select_list.append(select_idx) - select.append((col, None)) - select_idx += 1 klass_info = { "model": self.query.model, - "select_fields": select_list, + "select_fields": list( + range( + len(self.query.extra_select), + len(self.query.extra_select) + len(cols), + ) + ), } - for alias, annotation in self.query.annotation_select.items(): - annotations[alias] = select_idx - select.append((annotation, alias)) - select_idx += 1 + selected = [] + if self.query.selected is None: + selected = [ + *( + (alias, RawSQL(*args)) + for alias, args in self.query.extra_select.items() + ), + *((None, col) for col in cols), + *self.query.annotation_select.items(), + ] + else: + for alias, expression in self.query.selected.items(): + # Reference to an annotation. + if isinstance(expression, str): + expression = self.query.annotations[expression] + # Reference to a column. + elif isinstance(expression, int): + expression = cols[expression] + selected.append((alias, expression)) + + for select_idx, (alias, expression) in enumerate(selected): + if alias: + annotations[alias] = select_idx + select.append((expression, alias)) if self.query.select_related: related_klass_infos = self.get_related_selections(select, select_mask) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 07c3fdbd34..ce97ebe1d1 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -26,6 +26,7 @@ from django.db.models.expressions import ( Exists, F, OuterRef, + RawSQL, Ref, ResolvedOuterRef, Value, @@ -265,6 +266,7 @@ class Query(BaseExpression): # Holds the selects defined by a call to values() or values_list() # excluding annotation_select and extra_select. values_select = () + selected = None # SQL annotation-related attributes. annotation_select_mask = None @@ -584,6 +586,7 @@ class Query(BaseExpression): else: outer_query = self self.select = () + self.selected = None self.default_cols = False self.extra = {} if self.annotations: @@ -1194,13 +1197,10 @@ class Query(BaseExpression): if select: self.append_annotation_mask([alias]) else: - annotation_mask = ( - value - for value in dict.fromkeys(self.annotation_select) - if value != alias - ) - self.set_annotation_mask(annotation_mask) + self.set_annotation_mask(set(self.annotation_select).difference({alias})) self.annotations[alias] = annotation + if self.selected: + self.selected[alias] = alias def resolve_expression(self, query, *args, **kwargs): clone = self.clone() @@ -2153,6 +2153,7 @@ class Query(BaseExpression): self.select_related = False self.set_extra_mask(()) self.set_annotation_mask(()) + self.selected = None def clear_select_fields(self): """ @@ -2162,10 +2163,12 @@ class Query(BaseExpression): """ self.select = () self.values_select = () + self.selected = None def add_select_col(self, col, name): self.select += (col,) self.values_select += (name,) + self.selected[name] = len(self.select) - 1 def set_select(self, cols): self.default_cols = False @@ -2416,12 +2419,23 @@ class Query(BaseExpression): if names is None: self.annotation_select_mask = None else: - self.annotation_select_mask = list(dict.fromkeys(names)) + self.annotation_select_mask = set(names) + if self.selected: + # Prune the masked annotations. + self.selected = { + key: value + for key, value in self.selected.items() + if not isinstance(value, str) + or value in self.annotation_select_mask + } + # Append the unmasked annotations. + for name in names: + self.selected[name] = name self._annotation_select_cache = None def append_annotation_mask(self, names): if self.annotation_select_mask is not None: - self.set_annotation_mask((*self.annotation_select_mask, *names)) + self.set_annotation_mask(self.annotation_select_mask.union(names)) def set_extra_mask(self, names): """ @@ -2440,6 +2454,7 @@ class Query(BaseExpression): self.clear_select_fields() self.has_select_fields = True + selected = {} if fields: field_names = [] extra_names = [] @@ -2448,13 +2463,16 @@ class Query(BaseExpression): # Shortcut - if there are no extra or annotations, then # the values() clause must be just field names. field_names = list(fields) + selected = dict(zip(fields, range(len(fields)))) else: self.default_cols = False for f in fields: - if f in self.extra_select: + if extra := self.extra_select.get(f): extra_names.append(f) + selected[f] = RawSQL(*extra) elif f in self.annotation_select: annotation_names.append(f) + selected[f] = f elif f in self.annotations: raise FieldError( f"Cannot select the '{f}' alias. Use annotate() to " @@ -2466,13 +2484,13 @@ class Query(BaseExpression): # `f` is not resolvable. if self.annotation_select: self.names_to_path(f.split(LOOKUP_SEP), self.model._meta) + selected[f] = len(field_names) field_names.append(f) self.set_extra_mask(extra_names) self.set_annotation_mask(annotation_names) - selected = frozenset(field_names + extra_names + annotation_names) else: field_names = [f.attname for f in self.model._meta.concrete_fields] - selected = frozenset(field_names) + selected = dict.fromkeys(field_names, None) # Selected annotations must be known before setting the GROUP BY # clause. if self.group_by is True: @@ -2495,6 +2513,7 @@ class Query(BaseExpression): self.values_select = tuple(field_names) self.add_fields(field_names, True) + self.selected = selected if fields else None @property def annotation_select(self): @@ -2508,9 +2527,9 @@ class Query(BaseExpression): return {} elif self.annotation_select_mask is not None: self._annotation_select_cache = { - k: self.annotations[k] - for k in self.annotation_select_mask - if k in self.annotations + k: v + for k, v in self.annotations.items() + if k in self.annotation_select_mask } return self._annotation_select_cache else: |
