Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#33770 closed New feature (wontfix)

Add bulk_update() support for unique fields instead of only primary key

Reported by: Ebram Shehata Owned by: nobody
Component: Database layer (models, ORM) Version: 4.0
Severity: Normal Keywords: models, orm
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

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

Change History (3)

comment:1 by Carlton Gibson, 2 years ago

Needs documentation: set
Needs tests: set
Patch needs improvement: set
Resolution: wontfix
Status: newclosed

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.

in reply to:  1 comment:2 by Ebram Shehata, 2 years ago

Replying to 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.

Version 0, edited 2 years ago by Ebram Shehata (next)

comment:3 by Carlton Gibson, 2 years ago

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.

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