Opened 4 years ago

Closed 3 years ago

#17644 closed Cleanup/optimization (fixed)

Use namedtuples in Query.alias_map to make debugging easier

Reported by: lrekucki Owned by: adrian
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 lrekucki 4 years ago.
ticket_17644.diff (15.2 KB) - added by akaariai 3 years ago.

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by lrekucki

comment:1 Changed 4 years ago by lrekucki

  • Has patch set
  • Owner changed from nobody to lrekucki

comment:2 Changed 4 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

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 3 years ago by akaariai

  • 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 3 years ago by akaariai

comment:4 Changed 3 years ago by adrian

  • Owner changed from lrekucki to adrian
  • Status changed from new to assigned

Looking into this now...

comment:5 Changed 3 years ago by Alex

LGTM.

comment:6 Changed 3 years ago by adrian

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.
Back to Top