diff options
| author | Simon Charette <charette.s@gmail.com> | 2023-01-06 09:10:16 -0500 |
|---|---|---|
| committer | Mariusz Felisiak <felisiak.mariusz@gmail.com> | 2023-01-09 10:52:51 +0100 |
| commit | dd68af62b2b27ece50d434f6a351877212e15c3f (patch) | |
| tree | ca7b43eebd1198c016d3d50a3ade216614237179 /django/db/models/sql | |
| parent | 016bead6a23989adec5c7ee6948db7ce2fc5e89b (diff) | |
Fixed #34176 -- Fixed grouping by ambiguous aliases.
Regression in b7b28c7c189615543218e81319473888bc46d831.
Refs #31377.
Thanks Shai Berger for the report and reviews.
test_aggregation_subquery_annotation_values_collision() has been
updated as queries that are explicitly grouped by a subquery should
always be grouped by it and not its outer columns even if its alias
collides with referenced table columns. This was not possible to
accomplish at the time 10866a10 landed because we didn't have compiler
level handling of colliding aliases.
Diffstat (limited to 'django/db/models/sql')
| -rw-r--r-- | django/db/models/sql/compiler.py | 39 | ||||
| -rw-r--r-- | django/db/models/sql/query.py | 45 |
2 files changed, 39 insertions, 45 deletions
diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py index 41c5c9044f..97ecd58bdd 100644 --- a/django/db/models/sql/compiler.py +++ b/django/db/models/sql/compiler.py @@ -123,7 +123,7 @@ class SQLCompiler: if self.query.group_by is None: return [] expressions = [] - allows_group_by_refs = self.connection.features.allows_group_by_refs + group_by_refs = set() if self.query.group_by is not True: # If the group by is set to a list (by .values() call most likely), # then we need to add everything in it to the GROUP BY clause. @@ -133,21 +133,23 @@ class SQLCompiler: for expr in self.query.group_by: if not hasattr(expr, "as_sql"): expr = self.query.resolve_ref(expr) - if not allows_group_by_refs and isinstance(expr, Ref): - expr = expr.source - expressions.append(expr) + if isinstance(expr, Ref): + if expr.refs not in group_by_refs: + group_by_refs.add(expr.refs) + expressions.append(expr.source) + else: + expressions.append(expr) # Note that even if the group_by is set, it is only the minimal # set to group by. So, we need to add cols in select, order_by, and # having into the select in any case. - ref_sources = {expr.source for expr in expressions if isinstance(expr, Ref)} - aliased_exprs = {} - for expr, _, alias in select: - # Skip members of the select clause that are already included - # by reference. - if expr in ref_sources: - continue + selected_expr_indices = {} + for index, (expr, _, alias) in enumerate(select, start=1): if alias: - aliased_exprs[expr] = alias + selected_expr_indices[expr] = index + # Skip members of the select clause that are already explicitly + # grouped against. + if alias in group_by_refs: + continue expressions.extend(expr.get_group_by_cols()) if not self._meta_ordering: for expr, (sql, params, is_ref) in order_by: @@ -162,14 +164,21 @@ class SQLCompiler: seen = set() expressions = self.collapse_group_by(expressions, having_group_by) + allows_group_by_select_index = ( + self.connection.features.allows_group_by_select_index + ) for expr in expressions: - if allows_group_by_refs and (alias := aliased_exprs.get(expr)): - expr = Ref(alias, expr) try: sql, params = self.compile(expr) except (EmptyResultSet, FullResultSet): continue - sql, params = expr.select_format(self, sql, params) + if ( + allows_group_by_select_index + and (select_index := selected_expr_indices.get(expr)) is not None + ): + sql, params = str(select_index), () + else: + sql, params = expr.select_format(self, sql, params) params_hash = make_hashable(params) if (sql, params_hash) not in seen: result.append((sql, params)) diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py index 9e70aadca1..3fe00002bd 100644 --- a/django/db/models/sql/query.py +++ b/django/db/models/sql/query.py @@ -2211,40 +2211,25 @@ class Query(BaseExpression): primary key, and the query would be equivalent, the optimization will be made automatically. """ - if allow_aliases: - # Column names from JOINs to check collisions with aliases. - column_names = set() - seen_models = set() - for join in list(self.alias_map.values())[1:]: # Skip base table. - model = join.join_field.related_model - if model not in seen_models: - column_names.update( - {field.column for field in model._meta.local_concrete_fields} - ) - seen_models.add(model) - if self.values_select: - # If grouping by aliases is allowed assign selected values - # aliases by moving them to annotations. - group_by_annotations = {} - values_select = {} - for alias, expr in zip(self.values_select, self.select): - if isinstance(expr, Col): - values_select[alias] = expr - else: - group_by_annotations[alias] = expr - self.annotations = {**group_by_annotations, **self.annotations} - self.append_annotation_mask(group_by_annotations) - self.select = tuple(values_select.values()) - self.values_select = tuple(values_select) + if allow_aliases and self.values_select: + # If grouping by aliases is allowed assign selected value aliases + # by moving them to annotations. + group_by_annotations = {} + values_select = {} + for alias, expr in zip(self.values_select, self.select): + if isinstance(expr, Col): + values_select[alias] = expr + else: + group_by_annotations[alias] = expr + self.annotations = {**group_by_annotations, **self.annotations} + self.append_annotation_mask(group_by_annotations) + self.select = tuple(values_select.values()) + self.values_select = tuple(values_select) group_by = list(self.select) for alias, annotation in self.annotation_select.items(): if not (group_by_cols := annotation.get_group_by_cols()): continue - if ( - allow_aliases - and alias not in column_names - and not annotation.contains_aggregate - ): + if allow_aliases and not annotation.contains_aggregate: group_by.append(Ref(alias, annotation)) else: group_by.extend(group_by_cols) |
