[Django] #30828: Add ManyToMany relationships in bulk
Django
-------------------------------------+-------------------------------------
Reporter: David | Owner: David Foster
Foster |
Type: New | Status: assigned
feature |
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Given the following example model:
{{{
class M1(models.Model):
m2_set = models.ManyToManyField('M2')
}}}
It is already possible to associate one M1 with many M2s with a single DB
query:
{{{
m1.m2_set.add(*m2s)
}}}
However it's more difficult to associate many M1s with many M2s,
particularly if you want to skip associations that already exist:
{{{
# NOTE: Does NOT skip associations that already exist!
m1_and_m2_id_tuples = [(m1_id, m2_id), ...]
M1_M2 = M1.m2_set.through
M1_M2.objects.bulk_create([
M1_M2(m1_id=m1_id, m2_id=m2_id)
for (m1_id, m2_id) in
m1_and_m2_id_tuples
])
}}}
I propose adding the following APIs to bulk-associate relationships:
{{{
M1.m2_set.add_pairs(*[(m1, m2), ...], assert_no_collisions=False)
# --- OR ---
M1.m2_set.add_pair_ids(*[(m1_id, m2_id), ...], assert_no_collisions=False)
}}}
I also propose to add the following paired APIs to bulk-disassociate
relationships:
{{{
M1.m2_set.remove_pairs(*[(m1, m2), ...])
# --- OR ---
M1.m2_set.remove_pair_ids(*[(m1_id, m2_id), ...])
}}}
I have already written code for both of these cases and have been using it
in production for a few years. It probably needs to be extended to support
non-default database connections. Documentation+tests need to be added of
course.
Related thread on Django-developers:
https://groups.google.com/forum/#!topic/django-developers/n8ZN5uuuM_Q
API docstrings, with further details:
{{{
def add_pairs(
self: ManyToManyDescriptor, # M1.m2_set
m1_m2_tuples: 'List[Tuple[M1, M2]]',
*, assert_no_collisions: bool=False) -> None:
"""
Creates many (M1, M2) associations with O(1) database queries.
If any requested associations already exist, then they will be left
alone.
If you assert that none of the requested associations already exist,
you can pass assert_no_collisions=True to save 1 database query.
"""
def remove_pairs(
self: ManyToManyDescriptor, # M1.m2_set
m1_m2_tuples: 'List[Tuple[M1, M2]]') -> None:
"""
Deletes many (M1, M2) associations with O(1) database queries.
"""
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30828>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
| Foster
Type: New feature | Status: assigned
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
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
Comment:
I'll accept this given the preliminary discussion on the mailing list.
Let's see the patch. :)
Thanks David.
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:1>
Django
-------------------------------------+-------------------------------------
| Foster
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Patrick Cloke (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:2>
Django
-------------------------------------+-------------------------------------
| Foster
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Foster):
Draft documentation has been written. Implementation and tests are
pending. Will post PR once I have all three.
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:3>
Django
-------------------------------------+-------------------------------------
| Foster
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11899 PR] created.
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:4>
Django
-------------------------------------+-------------------------------------
| Foster
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
> However it's more difficult to associate many M1s with many M2s, ...
You can use `set()` on a reverse relationship, e.g.
{{{
m2.m1_set.add(*m1s)
}}}
Right?
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:5>
Django
-------------------------------------+-------------------------------------
| Foster
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by David Foster):
Yes you can bulk add on a reverse relationship to associate many items
with one other item in reverse. But you can't do some thing like adding
the relations {(a1, b1), (a1, b2), (a2, b3), (a4, b4)} all at once.
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:6>
Django
-------------------------------------+-------------------------------------
| Foster
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Simon Charette):
> But you can't do some thing like adding the relations ... all at once.
`.bulk_create(ignore_conflicts=True)`
[https://github.com/django/django/pull/11899#pullrequestreview-301073824
works reasonably well for this purpose] with a small boilerplate increase
at the profit of readability. The latter also works for manually defined
`through` with fields without defaults.
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:7>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: New feature => Cleanup/optimization
* component: Database layer (models, ORM) => Documentation
Comment:
Let's change this to a documentation issue. I think we should add a
paragraph about adding `ManyToManyField`'s relations with `bulk_create()`
for manually defined `through` and for different objects (on LHS and RHS)
to the "Insert in bulk" section (`docs/topics/db/optimization.txt`).
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:8>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Type: | Foster
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:9>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Type: | Foster
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:9>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Type: | Foster
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
Comment:
I have a new patch in [https://github.com/django/django/pull/11948 PR
beta] that provides a documentation workaround. Comments requested.
The proposed documentation recommends that users inline a fair bit of
boilerplate to perform bulk-associate and bulk-disassociate operations,
which feels a bit verbose to me, so I suggest still considering an
approach where we add dedicated methods for these operations.
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:10>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Type: | Foster
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:11>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Type: | Foster
Cleanup/optimization | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
Patch revised and ready for another review.
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:12>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Type: | Foster
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6a04e69e686cf417b469d7676f93c2e3a9c8d6a3" 6a04e69e]:
{{{
#!CommitTicketReference repository=""
revision="6a04e69e686cf417b469d7676f93c2e3a9c8d6a3"
Fixed #30828 -- Added how to remove/insert many-to-many relations in bulk
to the database optimization docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:13>
Django
and for manually defined "through".
-------------------------------------+-------------------------------------
Reporter: David Foster | Owner: David
Type: | Foster
Cleanup/optimization | Status: closed
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"afde973061a3e6477f3c454f4471842d37e73494" afde9730]:
{{{
#!CommitTicketReference repository=""
revision="afde973061a3e6477f3c454f4471842d37e73494"
[2.2.x] Fixed #30828 -- Added how to remove/insert many-to-many relations
in bulk to the database optimization docs.
Backport of 6a04e69e686cf417b469d7676f93c2e3a9c8d6a3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30828#comment:14>