Opened 5 years ago

Closed 5 years ago

#17644 closed Cleanup/optimization (fixed)

Use namedtuples in Query.alias_map to make debugging easier

Reported by: Łukasz Rekucki Owned by: Adrian Holovaty
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords:
Cc: anssi.kaariainen@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Debugging the SQL compiler is not an easy task, and having to keep track of values in 8-tuples doesn't make it easier ;) There are probably more places that this could be applied to, not just alias_map, but it's a start. namedtuple has no extra memory or performance overhead (I didn't notice any change in the runtime of testsuite).

On Python 2.5, we just use a plain tuple (lookup code hasn't been changed so it just works).

Attachments (2)

ticket17644.diff (2.6 KB) - added by Łukasz Rekucki 5 years ago.
ticket_17644.diff (15.2 KB) - added by Anssi Kääriäinen 5 years ago.

Download all attachments as: .zip

Change History (8)

Changed 5 years ago by Łukasz Rekucki

Attachment: ticket17644.diff added

comment:1 Changed 5 years ago by Łukasz Rekucki

Has patch: set
Owner: changed from nobody to Łukasz Rekucki

comment:2 Changed 5 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

I'd much prefer waiting until we can use namedtuple comprehensively, and take advantage of the attribute syntax for items, so I'd rather wait for 1.5 to do this.

comment:3 Changed 5 years ago by Anssi Kääriäinen

Cc: anssi.kaariainen@… added

The attached patch converts alias_map values from ordinary tuples to namedtuples. All tests passed (although I can't test geodjango currently). No speed regressions spotted using djangobench. The first attached patch contains some comments which would be useful to consolidate into this patch.

Changed 5 years ago by Anssi Kääriäinen

Attachment: ticket_17644.diff added

comment:4 Changed 5 years ago by Adrian Holovaty

Owner: changed from Łukasz Rekucki to Adrian Holovaty
Status: newassigned

Looking into this now...

comment:5 Changed 5 years ago by Alex Gaynor

LGTM.

comment:6 Changed 5 years ago by Adrian Holovaty

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top