Code

Opened 7 years ago

Closed 6 years ago

#3030 closed enhancement (duplicate)

unique=True and db_index=True leads to duplicated indexes

Reported by: fte@… Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: trivial Keywords: db-be-api manage.py sqlall unique db_index
Cc: gabor@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

At least in MySQL this produces a doubled index.

Example:

class Kunde(models.Model):
    kundenid  = models.AutoField(primary_key=True)
    loginname = models.CharField(unique=True, maxlength=50, db_index=True)

produces:

CREATE TABLE `mgt_kunden` (
    `kundenid`integer AUTO_INCREMENT NOT NULL PRIMARY_KEY,
    `loginname` varchar(50) NOT NULL UNIQUE
);
CREATE UNIQUE INDEX mgt_kunden_loginname ON `mgt_kunden` (`loginname`);

So there is the automatically generated index from the UNIQUE keyword
and the additional index requested by db_index=True.

Attachments (1)

3030.diff (751 bytes) - added by Peter Melvyn <peter.melvyn@…> 6 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 7 years ago by anonymous

ghaloo
class Kunde(models.Model):

kundenid = models.AutoFi

is it at [url=http://shurl.org/SGqxN]abba[/url] mkay?!?!
Models: Use of unique=True AND db_index=True at the sa
Models: Use of unique=True AND db_index=True at the same field should be flagged as an error
Status: new
Reported by: fte@… Assigned to: adrian
Priority: low Milestone: Version 1.0
Component: django-admin.py Version: SVN
Severity: trivial Keywords: manage.py sqlall
Cc:

At least in MySQL this produces a doubled index.

Example:

class Kunde(models.Model):

kundenid = models

comment:2 Changed 7 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

comment:3 Changed 7 years ago by torne-django@…

  • Summary changed from Models: Use of unique=True AND db_index=True at the same field should be flagged as an error to Models: unique=True should override db_index=True on (at least) MySQL

This definately generates incorrect SQL under MySQL. The UNIQUE constraint on the field itself produces an index named after the field, and the index created is named tablename_fieldname, so the result is two indexes. Using them both shouldn't be an error, I don't think - instead, the CREATE INDEX can simply be omitted (as an index is implicitly created by the UNIQUE constraint). No idea about other DB backends though.

comment:4 Changed 7 years ago by Simon G. <dev@…>

  • Keywords unique db_index added
  • Summary changed from Models: unique=True should override db_index=True on (at least) MySQL to unique=True and db_index=True leads to duplicated indexes
  • Triage Stage changed from Unreviewed to Accepted

Both MySQL and SQLite doubles the index (can someone confirm on Postgres?), but doesn't complain, so you end up with these indexes:

 PRIMARY  	 PRIMARY  	 0   	 Edit   	 Drop   	 kundenid
loginname 	UNIQUE 	0  	Edit 	Drop 	loginname
t3030_kunde_loginname 	UNIQUE 	0  	Edit 	Drop 	loginname

This is fairly trivial, since it works, but does increase the key size (and therefore database size etc) and is not terribly elegant.

comment:5 Changed 7 years ago by Simon G. <dev@…>

Actually - should db_index be creating a UNIQUE index at all? shouldn't it just be creating an INDEX (which is quite different)

comment:6 Changed 7 years ago by mtredinnick

Simon G: the db_index attribute checks to see if the index should be unique or not (so it examines the "unique" attribute as well). See django/core/management.py at the top of the get_sql_indexes_for_model() function for the details.

I think the small bug here is that the original code didn't take into account the fact that a "unique" constraint will construct an index in (almost) all databases because it is an efficient way of checking for uniqueness. So we may be able to short-circuit the index creation in those cases.

A fix for this ticket needs to check the behaviour of MySQL, PostgreSQL, SQLite (and preferably get some feedback on Oracle's behaviour as well) and confirm that they all create an index for "unique" fields. From memory, the answer is "yes", but I haven't explicitly checked it.

comment:7 Changed 7 years ago by bouldersprinters

Oracle behaves the same way, emitting a (benign) error when it tries to create the redundant index.

comment:8 Changed 7 years ago by Gábor Farkas <gabor@…>

  • Cc gabor@… added

this is a problem with PostgreSQL too.

also, this is probably obvious, but nevertheless..

not just exlicit "db_index=True" fields are a problem (where as a 'fix' you can simply omit the db_index=True)

having a SlugField(unique=True) has the same problem, and in that case, there's no way to avoid it (except deleting the index manually in the database

comment:9 Changed 7 years ago by ikelly

  • Resolution set to worksforme
  • Status changed from new to closed

I'm closing this because I believe it was fixed by the addition of the autoindexes_primary_keys backend feature back when the Oracle branch was merged in. Please re-open if that's not the case.

comment:10 Changed 6 years ago by Peter Melvyn

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I've test in r7411 and the problem persists. if unique=True and db_index=True, there is still redundant CREATE UNIQUE INDEX .... SQL command, because of MySQL creates on UNIQUE column declaration inherent one.

comment:11 Changed 6 years ago by Peter Melvyn <peter.melvyn@…>

I verified on MySQL 5.0.37 using InnoDB engine that setting DatabaseFeature.autoindexes_primary_keys to True really solves this problem. But in the backend.mysql.base, this feature is set to False. I didn't care about MySQL before version 5.x so I don't know what features have its older versions.

Changed 6 years ago by Peter Melvyn <peter.melvyn@…>

comment:12 Changed 6 years ago by Peter Melvyn <peter.melvyn@…>

  • Has patch set
  • Needs tests set

Because I don't know what was the behaveour of MySQL before version 5, I set DatabaseFeature?.autoindexes_primary_keys to True from MySQL server version 5++.

comment:13 Changed 6 years ago by ramiro

See also #5680 that takes the approach of simply doing away with the autoindexes_primary_key DatabaseFeatures property.

comment:14 Changed 6 years ago by Peter Melvyn <peter.melvyn@…>

Sure: In #5680 "Oracle guys" have introduced the new DB Feature autoindexes_primary_key and used it in logic generating indeces. But they left its value for MySQL equal to False, hence it has no effect. My patch sets its value to True, if version of MySQL server (read in run-time) is >= 5. This is from backward compatibility reason, because I've ignored MySQL before version 5 per se and I have no expirience, what happen if this feature would be set True while running obsolete MySQL version.

comment:15 Changed 6 years ago by ramiro

  • Keywords db-be-api added

comment:16 Changed 6 years ago by ramiro

  • Component changed from django-admin.py to Database wrapper

comment:17 Changed 6 years ago by mtredinnick

  • Resolution set to duplicate
  • Status changed from reopened to closed

I'm going to close this in favour of the approach in #5680. Less options are better and all backends behave the same here (even MySQL 4.x).

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


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

 
Note: See TracTickets for help on using tickets.