Opened 9 years ago

Closed 9 years ago

Last modified 6 years ago

#7447 closed (fixed)

QuerySet.update() bypasses custom methods

Reported by: Johannes Dollinger Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qsrf-cleanup
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


This is symmetrical to #6915 (and therefore should be handled simmilar). Options:

  • iterate the queryset, update and save() each instance individually. Because this would also work for sliced querysets #7298 would deserve a better fix then.
  • assert not self.has_custom_save(), "Cannot update a query for a model with a custom save() method.". This will probably break ManyRelated managers.
  • only add a big warning to QuerySet.update() and doc. This doesn't help at all to solve the issue.

+1 for the first option.

Change History (9)

comment:1 Changed 9 years ago by Johannes Dollinger

Basically the same issue: How will QuerySet.update() work with model validation (#6845)?

comment:2 Changed 9 years ago by Johannes Dollinger

Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 9 years ago by Jacob

milestone: 1.0

comment:4 Changed 9 years ago by anonymous

If update() is changed to iterate over every model, it's going to be much, much slower for querysets with thousands or more records.

Is there a way to check for a custom save method in the model, and only iterate if a custom save exists?

comment:5 Changed 9 years ago by anonymous

Alternatively, add a new method, similar to update (let's call it "queryset.save_data()" for example), and guarantee that that iterates no matter what, and leave the update() method alone.

Then, clearly document the fact that update() bypasses custom saves and is fast, save_data() uses custom saves but is slow.

comment:6 Changed 9 years ago by Johannes Dollinger

It should be possible to detect a custom update() method. I attempted to the same for delete() in my patch for #6915. But it's pretty much untested and therefore probably broken ..

comment:7 Changed 9 years ago by Malcolm Tredinnick

Changing the current behaviour isn't going to happen. The update() method is intended to be a bulk SQL operation. If you want to call the save() method on every item in the queryset, loop over the queryset. It's two lines of code. I will commit an update to the documentation for update() to mention that it's a direct SQL operation.

(This isn't quite the same as delete() for internal implementation reasons, so #6915 is still open for that reason.)

comment:8 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

(In [7884]) Documented that the update() method on querysets is a direct SQL call, not the
same as looping over the queryset and calling save() on each item (which is
less efficient). Fixed #7447.

comment:9 Changed 6 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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