[Django] #33770: Add bulk_update() support for unique fields instead of only primary key
Django
-------------------------------------+-------------------------------------
Reporter: Ebram | Owner: nobody
Shehata |
Type: New | Status: new
feature |
Component: Database | Version: 4.0
layer (models, ORM) |
Severity: Normal | Keywords: models, orm
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Currently, `bulk_update()` function from `django.db.models.query` only
performs the update using the primary key field..
I think we can generalize it more to use a unique field..
Example: `MyModel.objects.bulk_update(objs, fields=["name"],
unique_field="national_id")`
This will use `MyModel.national_id` to identify objects since
`national_id` is unique.
I wrote the code for it and I thought to contribute it to Django..
I also have a question about that function, we can't use it to update
primary keys.. I'm wondering why ? I want to also add that support to it.
Note: I'm not really sure what to do to reserve the implementation for me.
I want to be the one to implement it, actually, I already wrote the code.
Here's a PR I created: https://github.com/django/django/pull/15764
--
Ticket URL: <https://code.djangoproject.com/ticket/33770>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: closed
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: models, orm | Triage Stage:
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* needs_better_patch: 0 => 1
* resolution: => wontfix
* needs_tests: 0 => 1
* needs_docs: 0 => 1
Comment:
Hi Ebram. Welcome. Thanks!
Can I ask you to send this to the DevelopersMailingList to get more eyes
on it, and see if it's a change worth adding?
(I'll close the ticket now, but we'll reopen if there's a consensus to add
the feature.)
When you post, can you add a bit more as to why you think this is useful?
I can't immediately see why it matters, given that I have the objects in
hand 🤔
> I also have a question about that function, we can't use it to update
primary keys.. I'm wondering why ? I want to also add that support to it.
The mailing list is a better place to discuss this as well.
>...what to do to reserve the implementation for me...
If there's agreement to add, you can assign the ticket to yourself.
(There's not often contention in these areas 🙂)
Ref the PR: I appreciate this is just a first proof-of-concept but, all
changes need test coverage and docs.
--
Ticket URL: <https://code.djangoproject.com/ticket/33770#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Ebram Shehata | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: models, orm | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Ebram Shehata):
Replying to [comment:1 Carlton Gibson]:
Hi Carlton, thank you for replying!
Here's a discussion I started https://groups.google.com/g/django-
developers/c/SKvpdIN-NE0
I mentioned the use case in which I needed such feature and demonstrated
why this could be a useful addition. Please have a look. Thank you.
> Hi Ebram. Welcome. Thanks!
>
> Can I ask you to send this to the DevelopersMailingList to get more eyes
on it, and see if it's a change worth adding?
> (I'll close the ticket now, but we'll reopen if there's a consensus to
add the feature.)
>
> When you post, can you add a bit more as to why you think this is
useful? I can't immediately see why it matters, given that I have the
objects in hand 🤔
>
> > I also have a question about that function, we can't use it to update
primary keys.. I'm wondering why ? I want to also add that support to it.
>
> The mailing list is a better place to discuss this as well.
>
> >...what to do to reserve the implementation for me...
>
> If there's agreement to add, you can assign the ticket to yourself.
(There's not often contention in these areas 🙂)
>
> Ref the PR: I appreciate this is just a first proof-of-concept but, all
changes need test coverage and docs.
--
Ticket URL: <https://code.djangoproject.com/ticket/33770#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Ebram Shehata | Owner: nobody
Type: New feature | Status: closed
Component: Database layer | Version: 4.0
(models, ORM) |
Severity: Normal | Resolution: wontfix
Keywords: models, orm | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson):
Hi Ebram.
Yes, I'm following the thread there.
Jerch's reply seems positive: "Overall this sounds like a valuable API
addition to bulk_update…" but he raises the complexity of the MTI case,
and also whether it's needed:
> NB: Btw fetching proper pks upfront from some other unique field is
> typically very cheap compared to bulk_update runtime itself, given you
> have indexed those columns.
Short of further input, if you're keen to keep working on this, I'd
suggest adding some tests for the MTI case, and making that work. From
there — assuming it's not too complex -- you have a decent case for a
quality of life improvement.
--
Ticket URL: <https://code.djangoproject.com/ticket/33770#comment:3>