Opened 9 years ago

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

Download all attachments as: .zip

Change History (7)

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

Attachment: unique.diff added

Patch doing both of the things mentioned in the article

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

Attachment: unique.2.diff added

Patch doing both of the things mentioned in the ticket

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

Has patch: set

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 9 years ago by Simon G <dev@…>

Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 9 years ago by Malcolm Tredinnick

Triage Stage: Design decision neededAccepted

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 8 years ago by Ramiro Morales

Keywords: db-be-api added

comment:5 Changed 8 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

(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