Opened 3 years ago

Last modified 3 weeks ago

#32406 assigned New feature

Allow QuerySet.update() to return fields on supported backends.

Reported by: Tom Carrick Owned by: Aivars Kalvāns
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Diego Lima, Johannes Maron, Michael Rosner, John Speno Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Mailing list

For example:

Foo.objects.update(x="abc", returning=["pk", "x"])

To return something like:

[
    {"pk": 1, "x": "abc"},
    {"pk": 2, "x": "abc"},
    {"pk": 3, "x": "abc"},
]

The exact API and implementation is still a little unclear, but there seems to be support for doing something.

Change History (20)

comment:1 by Tom Carrick, 3 years ago

Owner: changed from nobody to Tom Carrick

comment:2 by Carlton Gibson, 3 years ago

Triage Stage: UnreviewedAccepted

OK, I'll provisionally accept on the basis of the discussion.
It would be good if we could pin down an API that was generally agreed upon (ref the return value seems the main sticking point) before implementation began.

comment:3 by Chris Jerdonek, 3 years ago

Recently, on issue #32381 about bulk_update(), I suggested the idea of returning different values as attributes of a single result object (e.g. a namedtuple). That was about bulk_update() rather than update(), but the principle is the same. In this case, the two possible values under consideration could be approximately named something like number_matched and values.

comment:4 by Diego Lima, 3 years ago

Cc: Diego Lima added

comment:5 by Johannes Maron, 3 years ago

Cc: Johannes Maron added

comment:6 by Johannes Maron, 3 years ago

Hi there, I implemented the returning support in the past. I believe this feature is possible, however, I think we need to be sure how this differs from setting db_returning on the fields itself. In case you didn't know (it's not documented yet) You have multiple return values by setting that attribute to true. I would be curious if you could provide a more detailed use case. Best, Joe

FYI, this might be somewhat related to #30032 and #470

comment:7 by Tom Carrick, 3 years ago

Johannes, I originally saw a couple of use-cases:

  1. There is some before update trigger that changes the data. If I understand it, db_returning should cover this (I had no idea it existed as it's not documented).
  2. You are creating an API (or not an API) with a bulk update feature, and you want to return the results to the user without making another query to gather them.

comment:8 by Johannes Maron, 3 years ago

Hi there,

Interesting cases. DB triggers make things slightly more difficult as the db_returning feature is currently only tested for inserts not updates.

I would see two things here, first, to add db_retuning support to a save call (single object update). Second, you could build on those API changes to add this functionality to update.
Honestly, with #470 on its way. It stands to reason, if it made sense to refresh and object from the database by default. But that's a mailing list discussion for future me ;)

In any event, this proposal seems justified to me. If you find the time to tackle it, I am happy to help out with reviews.

Best,
Joe

comment:9 by Tom Carrick, 2 years ago

Owner: Tom Carrick removed
Status: assignednew

comment:10 by Michael Rosner, 16 months ago

Cc: Michael Rosner added

comment:11 by Aivars Kalvāns, 7 months ago

Owner: set to Aivars Kalvāns
Status: newassigned

I have a proof of concept in my branch, I will clean up and open a PR this week. But I took a slightly different approach so I would like to get some feedback.

I added a new .update_returning method that returns a query set instead of changing the existing .update that returns a number. That allows us to do things like:

we can get the model after the update which is the main use case IMO. I have used this approach in accounting systems, there are some articles showing when it's useful https://hakibenita.com/django-concurrency#update-and-immediately-return

updated = (
            UpdateReturningModel.objects.filter(key=obj.key)
            .update_returning(hits=F("hits") + 1)
            .get()
        )

we can also do some lazy loading

        updated = (
            UpdateReturningModel.objects.filter(key=obj.key)
            .update_returning(hits=F("hits") + 1)
            .only("hits")
            .get()
        )

        updated = (
            UpdateReturningModel.objects.filter(key=obj.key)
            .update_returning(hits=F("hits") + 1)
            .defer("hits", "content")
            .get()
        )

or keep working without models

        updated = UpdateReturningModel.objects.update_returning(
            hits=F("hits") + 1
        ).values("pk", "hits")

        updated = UpdateReturningModel.objects.update_returning(
            hits=F("hits") + 1
        ).values_list("hits", flat=True)

comment:12 by Aivars Kalvāns, 7 months ago

Has patch: set

I created a PR

comment:13 by Mariusz Felisiak, 7 months ago

We don't have, and never need a separate method for INSERT ... RETURNING), so I'm not sure why you want to handle UPDATE ... RETURNING in a separate method.

comment:14 by Aivars Kalvāns, 7 months ago

Just because the update is returning the number of rows updated. I can do the same with Foo.objects.update(x="newx", returning=True) assuming no model will have a returning field. Whatever fits best, I just want to be able to do returning without executing a raw SQL.

in reply to:  14 comment:15 by Mariusz Felisiak, 7 months ago

Replying to Aivars Kalvāns:

Just because the update is returning the number of rows updated. I can do the same with Foo.objects.update(x="newx", returning=True) assuming no model will have a returning field. Whatever fits best, I just want to be able to do returning without executing a raw SQL.

Have you seen the mailing list discussion? There are some proposals to use the current methods without breaking backward compatibility, e.g. namedtuple.

comment:16 by Aivars Kalvāns, 7 months ago

Yes I saw it. But I did not see there a way to implement the new behavior without breaking the API. Is there something I missed? Returning a tuple with (n, [data]) will break the code that expects a simple integer, making the returning kwarg special also can break something (but unlikely) and chaining of method calls like .update().value() is not possible because the update is expected to execute the SQL before returning.

So asides from naming the function I think that returning models by default is nice because it includes PK and the result is "usable". If you don't need the models, you can ask for values or values_list. There are people using QuerySet.raw() to achieve similar result (https://hakibenita.com/django-concurrency#update-and-immediately-return)

comment:17 by Aivars Kalvāns, 7 months ago

Maybe doing Foo.objects.update(x="newx", returning=True) and returning a QuerySet would work better. I found only a single project with returning model field in github https://github.com/annalee/alienplanit/blob/b54722d683a5e8eba03f4467a367bcf24339bb32/submissions/models.py#L35

comment:18 by Mariusz Felisiak, 7 months ago

Please describe your proposition on the mailing list, where you'll reach a wider audience and see what other think.

comment:19 by John Speno, 7 months ago

Cc: John Speno added

comment:20 by Sarah Boyce, 3 weeks ago

Patch needs improvement: set

Marking "Patch needs improvement" as the API is being discussed and the current proposal in the discussion is different to the patch

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