#21249 closed Bug (fixed)
Typo in SQLCompiler.get_grouping()
| Reported by: | Michael Manfre | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | yes | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
From https://github.com/django/django/blob/master/django/db/models/sql/compiler.py#L578
for order, order_params in ordering_group_by:
# Even if we have seen the same SQL string, it might have
# different params, so, we add same SQL in "has params" case.
if order not in seen or params:
result.append(order)
params.extend(order_params)
seen.add(order)
should be:
for order, order_params in ordering_group_by:
# Even if we have seen the same SQL string, it might have
# different params, so, we add same SQL in "has params" case.
if order not in seen or order_params:
result.append(order)
params.extend(order_params)
seen.add(order)
Change History (5)
comment:1 by , 12 years ago
| Easy pickings: | set |
|---|---|
| Has patch: | set |
comment:2 by , 12 years ago
| Easy pickings: | unset |
|---|---|
| Needs tests: | set |
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 12 years ago
From IRC:
<manfre> This seems incorrect. https://github.com/django/django/blob/master/django/db/models/sql/compiler.py#L578 <manfre> Based upon the comment, shouldn't it check order_params instead of params? <akaariai> manfre: good catch. I have a strange feeling I know who is going to get git blame'd for that. <akaariai> and yes, it was me... ... <akaariai> manfre: the params -> order_params seems like an easy change, so that separately
comment:4 by , 12 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Note:
See TracTickets
for help on using tickets.
https://github.com/django/django/pull/1721