Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#17435 closed Bug (fixed)

document that QuerySet.update returns not the number of rows changed, but the number of rows matched

Reported by: m17.admin@… Owned by: nobody
Component: Documentation Version: 1.3
Severity: Normal Keywords:
Cc: estebistec@…, timograham@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It’s documented that QuerySet.update “returns the number of rows affected”.
Actual result — the number of rows matched.

I’m not sure, whether it’s a database layer bug, or just a documentation bug.

>>> Bar.objects.filter(pk=1).update(state=2)
1L
>>> Bar.objects.filter(pk=1).update(state=2)
1L
mysql> UPDATE foo_bar SET state=2 WHERE id=1;
Query OK, 1 row affected (0.02 sec)
Rows matched: 1  Changed: 1  Warnings: 0

mysql> UPDATE foo_bar SET state=2 WHERE id=1;
Query OK, 0 rows affected (0.01 sec)
Rows matched: 1  Changed: 0  Warnings: 0

Environment:

  • MySQL 5.1.58-1ubuntu-1
  • Django 1.3.1

Attachments (1)

17435.diff (1.3 KB) - added by timo 3 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 4 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to duplicate
  • Status changed from new to closed
  • Triage Stage changed from Unreviewed to Design decision needed

As far as I can tell, this is covered by #16891.

comment:2 Changed 4 years ago by estebistec

Hmm... I didn't make any changes along this line. So if update returns *matches* instead of *updates* then I think this is it's own bug. I'd be happy to role it into my bug and work on the changes there though, but maybe that's not the right thing to do?

comment:3 Changed 4 years ago by estebistec

  • Cc estebistec@… added

comment:4 Changed 4 years ago by kmtracey

I don't believe update returning matches rather than changed rows is a bug, but rather an intentional choice. #10348 reported MySQL as being out of line from the other DBs in returning changed rows, and the fix for it was to set a flag to make it fall in line with the others. I'm a bit surprised the documentation isn't more explicit that the number returned is matched, not necessarily changed. I don't believe we can, at this point, simply change the value returned here to be the other choice...

comment:5 Changed 4 years ago by sergeykolosov

Then, at least, it should be reflected in documentation. Shall I reopen this ticket as a documentation bug?

comment:6 Changed 4 years ago by kmtracey

  • Component changed from Database layer (models, ORM) to Documentation
  • Resolution duplicate deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Design decision needed to Accepted

Here's a django-dev thread where the return value from update() was discussed: http://groups.google.com/group/django-developers/browse_thread/thread/bea3c2a90e229f0f/

The update query return value is used internally by Django to decide if a model save(force_update=True) is successful: https://code.djangoproject.com/browser/django/tags/releases/1.3.1/django/db/models/base.py#L527

"Successful" for save with force_update=True simply means the row existed in the DB prior to the call, not that any data values were actually changed by the save(). Changing update to return the actually changed row count would cause a problem for this code. If the row existed but was not changed by the update() call then this code would start raising an exception where previously it did not.

Therefore I think the right fix here is to the docs, to make it clear that the return value is matched rows, not changed. (Current doc uses the word "affected" which I consider ambiguous, it's not saying one way or the other whether it means potentially affected or actually affected.)

comment:7 Changed 3 years ago by carljm

  • Summary changed from QuerySet.update returns not the number of rows affected, but the number of rows matched to document that QuerySet.update returns not the number of rows changed, but the number of rows matched

Changed 3 years ago by timo

comment:8 Changed 3 years ago by timo

  • Cc timograham@… added
  • Has patch set

comment:9 Changed 3 years ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

comment:10 Changed 3 years ago by Tim Graham <timograham@…>

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

In 6d46c740d80b0c7f75064bc6bb4d99b15b106ba4:

Fixed #17435 - Clarified that QuerySet.update returns the number of rows matched

comment:11 Changed 3 years ago by Tim Graham <timograham@…>

In c06b724a00236b086a5a64510d8bf11978f083f8:

[1.4.X] Fixed #17435 - Clarified that QuerySet.update returns the number of rows matched

Backport of 6d46c740d8 from master

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