Opened 8 years ago

Closed 7 years ago

#5671 closed (fixed)

Change SQL generation when primary_key=True and unique=True

Reported by: Nis Jørgensen <nis@…> Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: db-be-api
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

At the moment, these two models, which are logically equivalent, generate different sql:

class Country(models.Model):
    isocode = CharField(max_length=3, primary_key=True, unique=True)

class Country(models.Model):
    isocode = CharField(max_length=3, primary_key=True)
  • at least for backends other than Oracle

I would like to suggest that we do not create explicit UNIQUE constraints for (single field) primary keys.

On the other hand, I think it would be nice if field.unique returned True for a primary key field no matter if it was defined as such or not.

I will write a patch doing this, but am of course open to alternative suggestions.

Attachments (2)

unique.diff (4.8 KB) - added by Nis Jørgensen <nis@…> 8 years ago.
Patch doing both of the things mentioned in the article
unique.2.diff (4.8 KB) - added by Nis Jørgensen <nis@…> 8 years ago.
Patch doing both of the things mentioned in the ticket

Download all attachments as: .zip

Change History (7)

Changed 8 years ago by Nis Jørgensen <nis@…>

Patch doing both of the things mentioned in the article

Changed 8 years ago by Nis Jørgensen <nis@…>

Patch doing both of the things mentioned in the ticket

comment:1 Changed 8 years ago by Nis Jørgensen <nis@…>

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Oops - double submit. The patches are identical.

The patch does the following:

If a field is declared as primary_key, it is automatically flagged as unique

If a field is declared as primary_key, no UNIQUE is output in the SQL.

All checks of the type

if f.primary_key or f.unique:


are replaced by

if f.unique:

comment:2 Changed 8 years ago by Simon G <dev@…>

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 8 years ago by mtredinnick

  • Triage Stage changed from Design decision needed to Accepted

The concept seems reasonable. The patch introduces some random formatting changes that are not part of the change needed, but otherwise looks reasonable from just eyeballing it. I haven't tested it on all (or, in fact, any) db backends, so I'm not going to move it to almost ready-for-checkin just yet.

comment:4 Changed 7 years ago by ramiro

  • Keywords db-be-api added

comment:5 Changed 7 years ago by mtredinnick

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

(In [7790]) Make sure we only create the minimum number of table indexes for MySQL.

This patch simplifies a bunch of code for all backends and removes some
duplicate index creation for MySQL, in particular (versions 4.x and later).
Patch from Nis Jørgensen.

Fixed #5671, #5680, #7170, #7186.

Note: See TracTickets for help on using tickets.
Back to Top