Opened 13 years ago
Last modified 5 weeks ago
#17659 new Cleanup/optimization
django_admin_log searches are slow
Reported by: | anonymous | Owned by: | keeff |
---|---|---|---|
Component: | contrib.admin | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | anssi.kaariainen@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Queries against django_admin_log are slow because the soft-reference columns are not indexed.
Attachments (1)
Change History (10)
by , 13 years ago
Attachment: | django-logentry-index.diff added |
---|
comment:1 by , 13 years ago
Owner: | changed from | to
---|
comment:2 by , 13 years ago
Cc: | added |
---|
Adding some indexes makes sense to me. However, it would be nice to confirm both indexes are actually needed.
Is it possible to provide some data on how these indexes are used in the queries Django generates for realworld data? EXPLAIN ANALYZE from PostgreSQL would be perfect.
Multicolumn index could be the best solution here, but unfortunately that isn't currently supported. If possible, data for multicolumn index too would be great.
comment:3 by , 13 years ago
object_id is used in stock django, i.e. history_view() in contrib/admin/options.py. The index here is a clear win for any who use django_admin_log.
object_repr is often used in custom forms and other external code, one common example is to implement a global history search. Doing this via object_id is not reasonable since this would involve working out the object_id for all matching objects in every extant model, and even then it may not be viable to match history for deleted objects. Such forms should be efficient without needing a local modification to contrib/admin/models.py
comment:4 by , 13 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
I wonder if it is really necessary to have the index for object_repr alone though. Every django installation using admin would need to pay the price of that index, but it is not needed in stock installations. I have written multiple django apps, and never needed object_repr lookups. You can add the index by hand if needed.
So, I would say +1 on the object_id index, and more proof for the need of object_repr index.
Marking as accepted on the basis of need for object_id index. Although query plans with the index and without it are still missing.
comment:5 by , 9 years ago
This would be nice if it were to be completed some time in the not distant future.
However I think an index_together on content_type_id and object_id would be better as a search for object_id without knowing what type of object one is looking for is a strange query.
I could add a patch for this if it would move things along quicker. If I do, should I include a migration file?
comment:7 by , 9 years ago
I've just had to migrate our django_admin_log table (4.6 million rows) to add an index on (content_type_id, object_id) for the history view. I've also removed the plain content_type_id index since it is then a redundant as a prefix index.
Importantly, I had to change object_id to a varchar(255) since you can't index a text column on MySQL (idk about other DBs). You can index a prefix of up to 255 characters which would suffice (most object_ids are a handful of characters, being simple integers - being a TextField is presumably for completeness) - but Django models don't have this option yet...
comment:9 by , 5 weeks ago
@keeff Are you still interested in this issue? If not so, I'd like to claim it.
Patch to add indexes to django_admin_log