Opened 10 years ago

Closed 8 years ago

Last modified 3 years ago

#3182 closed enhancement (wontfix)

model instance update() method and QuerySet update_or_create() method

Reported by: Gary Wilson <gary.wilson@…> Owned by: Gary Wilson
Component: Database layer (models, ORM) Version: master
Severity: normal Keywords:
Cc: gary.wilson@…, real.human@…, Alexander Koshelev, George Sakkis Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Method would call get_or_create(), updating the object's data if the object already existed.

Idea came up in:

http://groups.google.com/group/django-developers/browse_thread/thread/49a6b99dbcea4364/

Attachments (8)

update_or_create.diff (1.4 KB) - added by Gary Wilson <gary.wilson@…> 10 years ago.
update_or_create() methods for QuerySet and Manager
update_or_create2.diff (7.0 KB) - added by Gary Wilson <gary.wilson@…> 10 years ago.
added documentation and tests
update_or_create3.diff (7.0 KB) - added by Gary Wilson <gary.wilson@…> 10 years ago.
removed the "required" wording in the documentation
update_and_update_or_create.diff (11.5 KB) - added by Gary Wilson <gary.wilson@…> 10 years ago.
update() from #3180 and update_or_create() together
3182-update_or_create-r9844.diff (12.1 KB) - added by Tai Lee 8 years ago.
Updated to work with r9844.
3182.diff (12.3 KB) - added by Gary Wilson 8 years ago.
took save() out of the assignment loop in update() and made a few tweaks to the docs.
3182.2.diff (12.3 KB) - added by Gary Wilson 8 years ago.
added versionadded directive for update_or_create
3182.update_or_create-only.3.diff (8.4 KB) - added by Antti Kaihola 5 years ago.
Adds .update_or_create() to QuerySet and Manager, without adding .update() to model instances. I attempted to make this atomic. Patch against git HEAD as of Jun 8 2012.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Cc: gary.wilson@… added
Summary: update_or_create() QuerySet method[patch] update_or_create() QuerySet method

Changed 10 years ago by Gary Wilson <gary.wilson@…>

Attachment: update_or_create.diff added

update_or_create() methods for QuerySet and Manager

comment:2 Changed 10 years ago by Gary Wilson <gary.wilson@…>

I should note that this patch depends on the update() method in #3181.

Changed 10 years ago by Gary Wilson <gary.wilson@…>

Attachment: update_or_create2.diff added

added documentation and tests

Changed 10 years ago by Gary Wilson <gary.wilson@…>

Attachment: update_or_create3.diff added

removed the "required" wording in the documentation

comment:3 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Correction: I should note that this patch depends on the update() method in #3180.

Changed 10 years ago by Gary Wilson <gary.wilson@…>

update() from #3180 and update_or_create() together

comment:4 Changed 10 years ago by anonymous

priority: normalhighest

comment:5 Changed 10 years ago by Gary Wilson <gary.wilson@…>

priority: highestnormal
Summary: [patch] update_or_create() QuerySet method[patch] model instance update() method and QuerySet update_or_create() method

Changed the title to reflect the merging of the two patches.

comment:6 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Summary: [patch] model instance update() method and QuerySet update_or_create() methodmodel instance update() method and QuerySet update_or_create() method
Triage Stage: UnreviewedReady for checkin

As per Jacob's comments in the mailing list.

comment:7 Changed 9 years ago by Gary Wilson

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Model instance update method should not use __dict__.update() as that won't work with fields like ForeignKey. Perhaps we need to loop through all the fields, using setattr and the other logic present in Model.__init__().

Changed 8 years ago by Tai Lee

Updated to work with r9844.

comment:8 Changed 8 years ago by Tai Lee

Cc: real.human@… added
Patch needs improvement: unset
Version: SVN

Updated the patch to work with trunk (r9844). Updated docs to match new docs. Updated the Model.update() method to use setattr() as mentioned above so it works with ForeignKey fields. Also updated a few typos in the existing QuerySet.update() tests and model instance docs.

This is listed as a maybe / "second-tier" feature for 1.1 with a comment that they are likely to be included if done. I think this should be ready to go in now.

comment:9 Changed 8 years ago by Tai Lee

milestone: 1.1 beta

Changed milestone to 1.1 beta, because this is listed as a 1.1 maybe feature that should be included if done, and it appears to be done.

comment:10 Changed 8 years ago by anonymous

Owner: changed from nobody to anonymous
Status: newassigned

comment:11 Changed 8 years ago by Gary Wilson

Owner: changed from anonymous to Gary Wilson
Status: assignednew

Changed 8 years ago by Gary Wilson

Attachment: 3182.diff added

took save() out of the assignment loop in update() and made a few tweaks to the docs.

Changed 8 years ago by Gary Wilson

Attachment: 3182.2.diff added

added versionadded directive for update_or_create

comment:12 Changed 8 years ago by Alexander Koshelev

Cc: Alexander Koshelev added

comment:13 Changed 8 years ago by Jacob

Resolution: wontfix
Status: newclosed

OK, this has gone around a few times on django-dev and in person, and it's clear that we're not reaching a consensus. There's a lot apathy, mostly -- few are arguing strongly for this feature, and few are arguing strongly against it. So, I'm going to do the BDFL thing and make a decision:

It's pretty clear to me that update() is a nonstarter; it's a single line of code::

def update(self, kw):

self.objects.filter(pk=self.pk).update(kw)

So that's out.

How about update_or_create()? I personally find the name counter-intuitive -- it's really get_and_then_update_or_create() -- but that's trivial; a better name could be found. A bigger problem is that it's nonatomic; we went to great lengths to insure that get_or_create() doesn't suffer from synchronization problems, but we can't do that here.

Given both these problems, and given that they won't exist if people are forced to write these methods in their own code, I'm marking this wontfix.

comment:14 in reply to:  13 Changed 7 years ago by George Sakkis

A bigger problem is that it's nonatomic; we went to great lengths to insure that get_or_create() doesn't suffer from synchronization problems, but we can't do that here.

Given both these problems, and given that they won't exist if people are forced to write these methods in their own code, I'm marking this wontfix.

Sorry for digging this up a year later but I didn't find an answer in django-dev; can you please clarify these last two points ? Why can't save(force_update=True) be in a transaction just like save(force_insert=True) and why the problem doesn't exist if the method is written outside the framework ?

comment:15 Changed 7 years ago by George Sakkis

Cc: George Sakkis added

comment:16 Changed 5 years ago by Jacob

milestone: 1.1 beta

Milestone 1.1 beta deleted

Changed 5 years ago by Antti Kaihola

Adds .update_or_create() to QuerySet and Manager, without adding .update() to model instances. I attempted to make this atomic. Patch against git HEAD as of Jun 8 2012.

comment:17 Changed 3 years ago by Tim Graham

Easy pickings: unset
UI/UX: unset

(#20429 is a duplicate of this ticket that has been accepted)

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