Opened 7 years ago

Closed 7 years ago

Last modified 7 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: russellm, mtredinnick Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


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 7 years ago.
prevent_update_queryset_r7599.patch (1.5 KB) - added by gav 7 years ago.
Prevents updates on querysets, patched against r7599

Download all attachments as: .zip

Change History (7)

Changed 7 years ago by sebastian_noack

comment:1 Changed 7 years ago by gav

  • Keywords qsrf-cleanup added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 7 years ago by gav

Prevents updates on querysets, patched against r7599

comment:2 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:3 Changed 7 years ago by emulbreh

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 7 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 7 years ago by emulbreh

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