Opened 11 years ago

Closed 5 years ago

#20581 closed New feature (fixed)

Support DEFERRABLE INITIALLY DEFERRED for UNIQUE constraints

Reported by: dmadeley@… Owned by: Ian Foote
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: db-indexes
Cc: Stephane "Twidi" Angel, python@…, aksheshdoshi@…, Ian Foote Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It would be useful, when using unique_together, to be able to defer constraint checking the way we defer foreign key checking until the end of the transaction.

Change History (22)

comment:1 by Anssi Kääriäinen, 11 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature
Version: 1.5master

Agreed that there are use cases where deferred unique constraints would be useful. However I am not sure if this needs to be part of Django's API (maybe it will be easy enough to create these constraints manually in the upcoming database migrations?).

This will be impossible to implement on those databases that do not support deferred unique constraints. I believe this includes both MySQL and SQLite of the core databases. I think this is a blocker for this ticket: if there is an API for unique deferred constraints in Django, then how does SQLite and MySQL interact with models that have these constraints?

I am marking this as accepted, but this should not be taken as indication that if patch is written it will be included in Django. This is accepted more in the sense that "yes, this seems useful in some cases, lets investigate if this should be part of Django".

comment:2 by dmadeley@…, 11 years ago

I was wondering if it could be added for UNIQUE in the backend when supported, the way the postgres backend currently does for foreign keys (i.e. always). Though INITALLY DEFERRED seems like something that more generically should be supported for foreign keys and other constraints via a deferred=True property on the field/meta and then a context manager for defer_constraints (which I think exists).

FWIW I did create the constraints I needed manually in my South migrations.

Probably a good first step is to break apart the create_model into more parts that the backend can override, and then adding DEFERRABLE to all postgres backend constraint adding methods (maybe after checking the version, since I think it requires a somewhat new postgres).

comment:3 by Anssi Kääriäinen, 11 years ago

I don't think it is a good idea to use DEFERRABLE INITIALLY DEFERRED for unique constaints. This will break backwards compatibility. The moment when unique constraint violation is raised is important, code can rightfully expect that unique constraint violations are raised on the moment of insert/update, not on commit.

It seems even DEFERRABLE INITIALLY IMMEDIATE can have negative results "To obtain standard-compliant behavior, declare the constraint as DEFERRABLE but not deferred (i.e., INITIALLY IMMEDIATE). Be aware that this can be significantly slower than immediate uniqueness checking." [www.postgresql.org/docs/9.0/static/sql-createtable.html]. So, creating all unique constraints as DEFERRABLE INITIALLY IMMEDIATE doesn't seem like a good option.

So, this needs some sort of API to inform that a deferrable constraint (initially immediate/not immediate) is needed. Maybe this could be achieved by having something like models.Unique(fields=('f1', 'f2'), deferrable=True) on the table (there might also be models.PrimaryKey(fields=('f1', 'f2')) at some point). Adding an API for unique_together seems complex, and adding a dedicated API just for deferred unique constraints doesn't feel right.

(FWIW I think foreign key constraints should be created as DEFERRABLE INITIALLY IMMEDIATE and switched to DEFERRED only when needed, eg for cascading deletes. It seems hard to actually get into this situation, this breaks existing code, and needs database migrations to work correctly).

I wonder if this should be marked "someday/maybe"...

comment:4 by Michal Petrucha, 10 years ago

My idea for an API was also to use something like models.Unique(*fields, deferrable=True, initially_deferred=True), but not as a Model class attribute, but rather inside unique_together:

class OrderedItem(models.Model):
    owner = ForeignKey('auth.User')
    order = PositiveIntegerField()
    data = TextField()

    class Meta:
        unique_together = (
            models.Unique('owner', 'order', deferrable=True),
        )

The idea is similar to urlpatterns – those also accept either a tuple, or the result of a url call. Also, it doesn't add a new API for this kind of constraints, merely extends the existing one.

comment:5 by Stephane "Twidi" Angel, 9 years ago

Cc: Stephane "Twidi" Angel added

comment:6 by Julien Hartmann, 9 years ago

Has there been any change on this issue?
If not, is there an approved workaround?

AFAIK, there is no way to customize the creation of the constraints in Django 1.9. As they cannot be altered, I have to manually drop and add them again every time django touches my table. There must be a better way.

comment:7 by Simon Charette, 9 years ago

I suppose this will be possible with the advanced index API.

comment:8 by Ian Foote, 9 years ago

Cc: python@… added

comment:9 by Akshesh Doshi, 9 years ago

Cc: aksheshdoshi@… added

comment:10 by Tim Graham, 8 years ago

Keywords: db-indexes added

comment:11 by Jon Dufresne, 7 years ago

I suppose this will be possible with the ​advanced index API.

I was able to create a POC using class based indexes, however it doesn't integrate as well as unique_together. Model._get_unique_checks() which is called by Model.validate_unique() which is used to clean model forms, will make no attempt to validate unique constraints defined through a class based index. What do you think about adding a hook Index.get_unique_checks() -- to be called by Model._get_unique_checks() -- so constraints defined this way can be used to enforce unique constraints in model forms?

comment:12 by Jon Dufresne, 7 years ago

Has patch: set
Needs documentation: set

PR

I've added a WIP PR that uses the suggestion mentioned above. I'm looking for some feedback on the approach to make sure it fits with the goals of the class based index feature. If the general approach looks good, I'll follow through by finishing the PR with docs.

comment:13 by Ian Foote, 6 years ago

Cc: Ian Foote added

comment:14 by Ian Foote, 6 years ago

Owner: changed from nobody to Ian Foote
Status: newassigned

comment:15 by Asif Saifuddin Auvi, 6 years ago

Needs documentation: unset

comment:16 by Mariusz Felisiak, 6 years ago

comment:17 by Mariusz Felisiak, 6 years ago

Patch needs improvement: set

comment:18 by Ian Foote, 5 years ago

Patch needs improvement: unset

I've finally gotten around to giving this another look and I think this is ready for another review.

comment:19 by Mariusz Felisiak, 5 years ago

Patch needs improvement: set

comment:20 by Ian Foote, 5 years ago

Patch needs improvement: unset

comment:21 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:22 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In c226c6cb:

Fixed #20581 -- Added support for deferrable unique constraints.

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