Opened 13 years ago
Closed 8 years ago
#16220 closed Cleanup/optimization (fixed)
Add introspection for multicolumn indexes
Reported by: | Jeffrey Gelens | Owned by: | Jeffrey Gelens |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | dceu2011 db-indexes |
Cc: | jeffrey@…, hvdklauw@…, gert.vangool@…, charette.s@…, Ivan Virabyan, aksheshdoshi@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The current database introspection methods don't support multi column indexes.
It's difficult to add this functionality to the current get_indexes methods as it's difficult to include it the current returned data d, which is:
{u'group_id': {'primary_key': False, 'unique': False}, u'id': {'primary_key': True, 'unique': True}}
What hvdklauw, jgelens and charstring suggest is a (*cough*) "get_real_indexes" method.
Attachments (1)
Change History (24)
comment:1 by , 13 years ago
Cc: | added |
---|
comment:2 by , 13 years ago
As this is a non public API the result can be updated to include the multicolumn fields. South is using this introspection function too, it needs to be patched accordingly.
comment:3 by , 13 years ago
Cc: | added |
---|
comment:4 by , 13 years ago
Suggestion:
Old output:
New output:
{ u'table_name_pkey': { 'primary_key': False, 'unique': False, 'fields': ('id',) }, u'table_name_fieldname': { 'primary_key': False, 'unique': False, 'fields': ('somefield',) }, u'table_name_355fd33': { 'primary_key': False, 'unique': False, 'fields': ('somefield1', 'somefield2']) # a multicolumn index },
comment:5 by , 13 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:6 by , 13 years ago
Cc: | removed |
---|
comment:7 by , 13 years ago
Cc: | added |
---|
comment:8 by , 13 years ago
Keywords: | inspectdb added |
---|
comment:10 by , 13 years ago
Cc: | added |
---|
comment:11 by , 13 years ago
Got this working now:
{ u'id': { 'primary_key': False, 'unique': False, 'fields': ('id',) }, u'somefield': { 'primary_key': False, 'unique': True, 'fields': ('somefield',) }, u'somefield1': { 'primary_key': False, 'unique': False, 'fields': ('somefield1', 'somefield2') # a multicolumn index }, }
Btw, I didn't find any usage of get_indexes in south :-/
comment:12 by , 13 years ago
Owner: | changed from | to
---|
by , 13 years ago
Attachment: | get_indexes.diff added |
---|
Quick implementation for PostgreSQL, MySQL and SQLite
comment:13 by , 13 years ago
I did note some inconsistencies between the different backends. (SQLite didn't output unique indexes)
Now they do the same, still not sure about the key values, you can't use the field name as there might be multiple.
I don't have access or knowledge of Oracle, so someone who does should fix that; also haven't written tests because django by itself doesn't support adding indexes on multiple fields (need #5805 for that, which in turn needs this to do tests)
comment:14 by , 13 years ago
Cc: | added |
---|
comment:15 by , 13 years ago
Cc: | added |
---|
comment:16 by , 12 years ago
Has patch: | set |
---|---|
Needs tests: | set |
This blocks #5805 (or at least blocks writing tests that ensure #5805 is working).
I see there's a design decision needed. IMO this is a good addition--there's a solid use case for only needing to index fields in the context of others. Some queries always reference the same fields when filtering, e.g. first_name
+ last_name
.
comment:17 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
What design decision is expected?
comment:18 by , 12 years ago
I think the design decision was for the suggested output of get_indexes().
comment:19 by , 8 years ago
The get_constraints
method of the introspection currently returns something similar to this for all databases.
{ u'introspection_article_headline_pub_date_c749bad3_idx':{ 'index':True, 'primary_key':False, 'orders':['ASC', 'ASC'], 'foreign_key':False, 'unique':False, 'check':False, 'columns':[u'headline', u'pub_date'] }, '__primary__':{ 'index':False, 'primary_key':True, 'foreign_key':False, 'unique':False, 'check':False, 'columns':[u'id'] }, u'introspection_article_reporter_id_c7562f75':{ 'index':True, 'primary_key':False, 'orders':['ASC'], 'foreign_key':False, 'unique':False, 'check':False, 'columns':[u'reporter_id'] }, }
If I understand correctly, that means this ticket got resolved somewhere down the line.
---
I also found an introspection test for get_indexes
method with the following docstring
""" Test that multicolumn indexes are not included in the introspection results. """
which I found something contradicting to this ticket.
comment:20 by , 8 years ago
Keywords: | db-indexes added; inspectdb removed |
---|---|
Version: | 1.3 → master |
comment:21 by , 8 years ago
AFAIK, the fact that get_indexes
only return single column indexes was due to its "limited" utility in inspectdb
which was just to determine the primary key or unique property of individual fields.
If we can make inspectdb
use the get_constraints
result to determine primary key
and unique
properties of individual fields (should be doable), we might as well deprecate and eventually remove get_indexes
.
comment:22 by , 8 years ago
Cc: | added |
---|
comment:23 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Also to note, the current mysql implementation isn't even correct compared to the other implementations. (dunno about oracle, don't know it).
It get's all the indexes and then loops through them and adds them to the dict. Meaning the last mention of a fieldname is used to define the indexes is has.