Opened 23 months ago

Closed 23 months ago

Last modified 23 months ago

#21249 closed Bug (fixed)

Typo in SQLCompiler.get_grouping()

Reported by: manfre Owned by: nobody
Component: Database layer (models, ORM) Version: master
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 Changed 23 months ago by manfre

  • Easy pickings set
  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 23 months ago by timo

  • Easy pickings unset
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 23 months ago by manfre

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 Changed 23 months ago by Anssi Kääriäinen <akaariai@…>

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

In 86c248aa646183ef4a1cb407bb3e4cb597272f63:

Fixed #21249 -- variable name typo in compiler.get_grouping()

The typo could have consequences in exceptional cases, but I didn't
figure out a way to actually produce such a case, so not tests.

Report & patch by Michael Manfre.

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

In 6781dc624318fd03032efb3182266bae0a5c8fc0:

[1.6.x] Fixed #21249 -- variable name typo in compiler.get_grouping()

The typo could have consequences in exceptional cases, but I didn't
figure out a way to actually produce such a case, so not tests.

Report & patch by Michael Manfre.

Backport of 86c248aa64 from master

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