[patch] queryset delete should return number of rows matched

[Steven Cummings]

Splitting this off from ticket #16549...

Deep in the bowels of django.db the rows modified value from update and delete queries is ignored and discarded. For reasons discussed on ticket 16549, it would sometimes be useful to have access to that value.

Objective of this bug is to passively return rows-modified up through the call-stack and, ultimately, hopefully from each of:

• Model.save()
• Model.delete()
• QuerySet.update()
• QuerySet.delete()

with consideration for transaction control/mgmt.

Update from comment thread: queryset update already returns rows-matched (and this can't be changed to rows-changed without breaking other things). So it is only deletes that need this change.

[Steven Cummings]

Claiming to work on.

[Steven Cummings]

Finally got around to these simple changes:

• ​https://github.com/estebistec/django/commit/b48a87afc324f5546b6654fa7638e406b397c0d6
• ​https://github.com/estebistec/django/commit/28ace32980b370fd17ae35019bfe8d055c673684

If the core devs approve of these changes I can get to work on creating the proper patch for SVN and add doc changes.

[Anssi Kääriäinen]

An idea for the save() patch: return UPDATED, INSERTED, or None instead of 1/0. You could still do if obj.save(): (matches both UPDATED and INSERTED) but if need be, you could see if the object was indeed updated or inserted. The value would be the "outermost" value for multitable inherited models.

I wonder if it would be a good idea to return the amount of deletions per object type when deleting? I see at least three choices:

• Just a dict model_class -> delete count, for example in your test case this would be {R:1, S:2, T:2}
• Tuple total_objects_deleted, abovementioned dict: Example: (5, {R:1, S:2, T:2})
• Tuple amount_of_outer_objs, abovementioned dict: Example: (1, {R:1, S:2, T:2})

To me it seems these could be useful, especially the #2 format. This information can be gotten nearly for free. The inconvenience here is that you can't do if obj.delete(): as (0, {}) isn't False. Maybe this could be postponed for later inclusion with kwarg with_counts=True or something. Or maybe there is just no use case for this.

[Steven Cummings]

In case anybody is paying more attention here than on django-dev, I made an update to roll the changes back to just the internal query classes for now:

• ​https://github.com/estebistec/django/commit/f178b72af132dd1ba588b89fe6915f5e9ba841d0

[Steven Cummings]

Patch to changes so far from git clone

[Steven Cummings]

I haven't gotten much feedback on this ticket, but what I've gotten so far has convinced me to back off of any changes to the base Model API. UpdateQuery already returned rowcounts, which means that only DeleteQuery needed updating. So, the patch is pretty tiny as you can see.

[Steven Cummings]

Updating summary

[Aymeric Augustin]

#17435 was a duplicate.

[Sergey Kolosov]

Replying to estebistec:

I haven't gotten much feedback on this ticket, but what I've gotten so far has convinced me to back off of any changes to the base Model API. UpdateQuery already returned rowcounts, which means that only DeleteQuery needed updating. So, the patch is pretty tiny as you can see.

Since QuerySet.update returns not the number of rows affected, but the number of rows matched (as I’ve described it in #17435), I believe the UpdateQuery still needs to be patched.

[Steven Cummings]

Yeah, if there are no objections to rolling #17435 into this bug, I'll go back in and add QuerySet.update to the patch.

[Karen Tracey]

No, I don't think update() can be switched to start returning the actually-modified row count rather than matched row count. See #17435 for why, the fix for that one is to clarify the doc.

[Steven Cummings]

Replying to kmtracey:

No, I don't think update() can be switched to start returning the actually-modified row count rather than matched row count. See #17435 for why, the fix for that one is to clarify the doc.

So I need to take a closer look here too and see if the delete() counts can mirror update() in that way.