Opened 13 years ago

Closed 13 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 13 years ago.
ticket_17644.diff (15.2 KB ) - added by Anssi Kääriäinen 13 years ago.

Download all attachments as: .zip

Change History (8)

by Łukasz Rekucki, 13 years ago

Attachment: ticket17644.diff added

comment:1 by Łukasz Rekucki, 13 years ago

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

comment:2 by Alex Gaynor, 13 years ago

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 by Anssi Kääriäinen, 13 years ago

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.

by Anssi Kääriäinen, 13 years ago

Attachment: ticket_17644.diff added

comment:4 by Adrian Holovaty, 13 years ago

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

Looking into this now...

comment:5 by Alex Gaynor, 13 years ago

LGTM.

comment:6 by Adrian Holovaty, 13 years ago

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