[Django] #20581: Support DEFERRABLE INITIALLY DEFERRED for UNIQUE constraints
Django
----------------------------------------------+--------------------
Reporter: dmadeley@… | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20581>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* needs_better_patch: => 0
* needs_tests: => 0
* version: 1.5 => master
* needs_docs: => 0
* type: Uncategorized => New feature
* stage: Unreviewed => Accepted
Comment:
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".
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:1>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by dmadeley@…):
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).
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:2>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
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"...
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:3>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by koniiiik):
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`:
{{{
#!python
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:4>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Twidi):
* cc: Twidi (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:5>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by spectras):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:6>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
I suppose this will be possible with the
[https://github.com/django/deps/pull/6/files advanced index] API.
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:7>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ian-Foote):
* cc: python@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:8>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by akki):
* cc: aksheshdoshi@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:9>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* keywords: => db-indexes
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:10>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne):
> 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:11>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* needs_docs: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/8620 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:12>
Django
Reporter: dmadeley@… | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Ian Foote):
* cc: Ian Foote (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:13>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Ian Foote):
* owner: nobody => Ian Foote
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:14>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Asif Saifuddin Auvi):
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:15>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
[https://github.com/django/django/pull/10338 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:16>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:17>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ian Foote):
* needs_better_patch: 1 => 0
Comment:
I've finally gotten around to giving this another look and I think this is
ready for another review.
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:18>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:19>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: db-indexes | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ian Foote):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:20>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:21>
Django
Reporter: dmadeley@… | Owner: Ian Foote
Component: Database layer | Version: master
(models, ORM) |
Keywords: db-indexes | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c226c6cb3209122b6732fd501e2994c408dc258e" c226c6cb]:
{{{
#!CommitTicketReference repository=""
revision="c226c6cb3209122b6732fd501e2994c408dc258e"
Fixed #20581 -- Added support for deferrable unique constraints.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20581#comment:22>