Opened 10 years ago

Closed 10 years ago

Last modified 7 years ago

#7447 closed (fixed)

QuerySet.update() bypasses custom Model.save() 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:

Description

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 Model.save() doc. This doesn't help at all to solve the issue.

+1 for the first option.

Change History (9)

comment:1 Changed 10 years ago by Johannes Dollinger

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

comment:2 Changed 10 years ago by Johannes Dollinger

Triage Stage: UnreviewedDesign decision needed

comment:3 Changed 10 years ago by Jacob

milestone: 1.0

comment:4 Changed 10 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 10 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 10 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 10 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 10 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 7 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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