Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#7298 closed (fixed)

update() on a sliced query set updates all objects.

Reported by: Sebastian Noack Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qsrf-cleanup
Cc: Russell Keith-Magee, Malcolm Tredinnick Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


What do think following code will do?


No it won't update the first object in the model, it will update all objects. Although I expected that it updates the first object, we should just prevent the user from doing this. Because of (at least MySql) do not support offset for UPDATE. It supports limit but their might be DBMS which do not support both on UPDATE and if you think about this use case, it is a real corner case. But anyway the user should not destroy his data by accident. I have written a patch which adds the same sanity check to QuerySet.update() as already done when calling QuerySet.order_by().

Attachments (2)

0001-Preventing-update-on-a-sliced-QuerySet-7298.patch (1.5 KB) - added by Sebastian Noack 11 years ago.
prevent_update_queryset_r7599.patch (1.5 KB) - added by George Vilches 11 years ago.
Prevents updates on querysets, patched against r7599

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by Sebastian Noack

comment:1 Changed 11 years ago by George Vilches

Keywords: qsrf-cleanup added

I think this patch is the right approach, I've only expanded it slightly by adding it in to the other update-like method on querysets, and cleaned up the wording a bit.

Changed 11 years ago by George Vilches

Prevents updates on querysets, patched against r7599

comment:2 Changed 11 years ago by Jacob

Resolution: fixed
Status: newclosed

(In [7601]) Fixed #7298: prevent update() on sliced QuerySet since UPDATE doesn't reliably support LIMIT/OFFSET. Thanks, George Vilches.

comment:3 Changed 11 years ago by Johannes Dollinger

QuerySet.update() bypasses custom methods. If this is intended behaviour the docs should mention it. Otherwise there had to be code to iterate the queryset anyway (manually updating each object) that could be applied to sliced querysets as well.

comment:4 Changed 11 years ago by Sebastian Noack

Not only overriden save() methods are bypassed, also the pre_save() method of fields are ignored. But at first, this has nothing to do with this ticket. And at second, the update() method is not a shortcut for, following:

for obj in query_set: = 'bar'

The update() method is intended to be a low level API to the UPDATE sql statement, to enable updating multiple objects at once with a single query. Maybe the documentation should explain this more clearly, if required.

comment:5 Changed 11 years ago by Johannes Dollinger

Mhm. I thought this was symmetrical to #6915, which jacob seemed to agree should be fixed. I opened #7447 as I had not seen your comment yet. At least there should be a warning in the docs: "If you want to write reusable code, you better not use QuerySet.update()."

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