Code

Opened 3 years ago

Last modified 13 months ago

#16220 new Cleanup/optimization

Add introspection for multicolumn indexes

Reported by: jgelens Owned by: jgelens
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords: dceu2011 inspectdb
Cc: jeffrey@…, hvdklauw@…, gert.vangool@…, charette.s@…, ivan_virabyan 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)

get_indexes.diff (4.2 KB) - added by hvdklauw 2 years ago.
Quick implementation for PostgreSQL, MySQL and SQLite

Download all attachments as: .zip

Change History (19)

comment:1 Changed 3 years ago by hvdklauw

  • Cc hvdklauw added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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.

comment:2 Changed 3 years ago by jgelens

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.

Last edited 3 years ago by jgelens (previous) (diff)

comment:3 Changed 3 years ago by jgelens

  • Cc jeffrey@… added

comment:4 Changed 3 years ago by jgelens

Suggestion:

Old output:

{u'group_id': {'primary_key': False, 'unique': False},
 u'id': {'primary_key': True, 'unique': True}}

Possible new output:

{
    u'table_name_pkey': {
        'primary_key': False, 
        'unique': False, 
        'fields': ('id',)
    },
    u'table_name_somefield': {
        'primary_key': False, 
        'unique': False, 
        'fields': ('somefield',)
    },
    u'table_name_355fd33': {
        'primary_key': False, 
        'unique': False, 
        'fields': ('somefield1', 'somefield2')  # a multicolumn index
    },
}

Where the key of the primary hash is the index name like how it's represented in the DB.

Last edited 3 years ago by jgelens (previous) (diff)

comment:5 Changed 3 years ago by hvdklauw

  • Triage Stage changed from Unreviewed to Design decision needed

comment:6 Changed 3 years ago by hvdklauw

  • Cc hvdklauw removed

comment:7 Changed 3 years ago by hvdklauw

  • Cc hvdklauw@… added

comment:8 Changed 3 years ago by ramiro

  • Keywords inspectdb added

comment:9 Changed 3 years ago by hvdklauw

#5805 depends on this one

comment:10 Changed 2 years ago by gvangool

  • Cc gert.vangool@… added

comment:11 Changed 2 years ago by jgelens

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 Changed 2 years ago by jgelens

  • Owner changed from nobody to jgelens

Changed 2 years ago by hvdklauw

Quick implementation for PostgreSQL, MySQL and SQLite

comment:13 Changed 2 years ago by hvdklauw

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 Changed 2 years ago by charettes

  • Cc charette.s@… added

comment:15 Changed 2 years ago by ivan_virabyan

  • Cc ivan_virabyan added

comment:16 Changed 20 months ago by dloewenherz

  • 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 Changed 13 months ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

What design decision is expected?

comment:18 Changed 13 months ago by jgelens

I think the design decision was for the suggested output of get_indexes().

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from jgelens to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.