Opened 3 years ago

Closed 22 months ago

Last modified 22 months ago

#18333 closed Bug (fixed)

Aggregating an annotated field does not quote the alias column name

Reported by: manfre Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: aggregate, annotate
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Given the below code snippet from aggregation.BaseAggregateTestCase.test_aggregate_annotation:

vals = Book.objects.annotate(num_authors=Count("authors__id")).aggregate(Avg("num_authors"))

The generated SQL does not quote the annotated field "num_authors" when used with the aggregate function. Generated SQL is of the form:

SELECT AVG(num_authors) FROM (... subquery ...)

Attachments (2)

django-ticket18333.diff (508 bytes) - added by manfre 3 years ago.
django-ticket18333.2.diff (951 bytes) - added by manfre 3 years ago.

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by manfre

Changed 3 years ago by manfre

comment:1 Changed 3 years ago by manfre

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I'm not aware of any databases that need to quote '*', but just in case there are I provided two patches. The second preloads the cache with '*' to prevent quoting it. Most commonly encountered by COUNT(*). Regardless of which patch is chosen (if any), 3rd party backends can override quote_cache to either have or omit this initial alias.

comment:2 Changed 3 years ago by lukeplant

Is there a bug caused by this behaviour? I can't see it at the moment.

comment:3 Changed 3 years ago by akaariai

Seems like if you use a SQL reserved keyword as the key in annotate, then you will get a broken query. No testing done here.

comment:4 Changed 3 years ago by lukeplant

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

Ah, I see, I was just thinking about characters that can't be part of keyword arguments anyway, so thought there wasn't an issue.

comment:5 Changed 22 months ago by manfre

Updated pull request with test. https://github.com/django/django/pull/1674

comment:6 Changed 22 months ago by manfre

  • Needs tests unset

comment:7 Changed 22 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 9a041807fcde7ff7245696a743196d31ec7c7b5c:

Fixed #18333 - Quoted annotated column names

comment:8 Changed 22 months ago by Anssi Kääriäinen <akaariai@…>

In 9027da65d3590a3bd319490d78c86ef09cd04f9e:

Added '*' to quote_cache

The commit for #18333 missed quote_cache default value for *.
Refs #18333.

Note: See TracTickets for help on using tickets.
Back to Top