Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for select clause in bulk_update #2265

Closed
jonashaag opened this issue Sep 11, 2020 · 6 comments
Closed

Add support for select clause in bulk_update #2265

jonashaag opened this issue Sep 11, 2020 · 6 comments

Comments

@jonashaag
Copy link
Contributor

Add support for optional select clause in bulk_update, for example to allow for something like

foos = [... list of Foos ...]
# Only allow updates to x when x=0.
nchanged = Foo.bulk_update(foos, [Foo.x], q=(Foo.x == 0))
if nchanged != len(foos):
    raise Error("Some Foos had x != 0")

I quickly hacked this together like this, and it seems to work.

    def bulk_update(cls, model_list, fields, batch_size=None, q=None):
...

            query = (
                cls.update(update)
                   .where(cls._meta.primary_key.in_(id_list))
            )
            if q is not None:
                query = query.where(q)
            n += query.execute()
        return n
@coleifer
Copy link
Owner

coleifer commented Sep 11, 2020

Since you are already explicitly specifying the model_list this seems redundant.

@jonashaag
Copy link
Contributor Author

It's not – think of my use case as "defensive" bulk updating of an attribute, for example allow only changes to x if x wasn't ever set and still has its default value of 0.

I think this can be done in Django as follows, but I'm not entirely sure (haven't tried):

Foo.objects.filter(x=0).bulk_update(foos, ['x'])

As an alternative, what do you think about factoring out the Case builder logic from bulk_update into a public helper method so it's easier for people to write their own custom bulk update logic?

@coleifer
Copy link
Owner

I'm not clear why you would be using bulk_update in that case? Presumably a regular-old update() query would work better?

@jonashaag
Copy link
Contributor Author

jonashaag commented Sep 11, 2020

Because the x may be different for each Foo. Say we start with 3 Foo in the database, with x=0. We do some work that computes 3 new (different!) x values, and we want to update all of the Foos with the new x values, with a single SQL update statement and in a way that never updates any x that were updated by something else while we were computing the new x values (race conditions).

@coleifer
Copy link
Owner

A relational database provides atomicity and isolation, so presumably there's a different way to handle that. I think this is a rather obscure usage because it implies that you are updating values from a list of models, but also that some of those models may be "out-of-sync". This, to me, is not an API failure but rather a result of trying to shoehorn some logic into a process that would be better handled another way.

@jonashaag
Copy link
Contributor Author

Isn't the scenario simply a case of a more general version of optimistic locking as described in the docs? http://docs.peewee-orm.com/en/latest/peewee/hacks.html#optimistic-locking There we have a version column, and no bulk update; here we have a non-version column with bulk update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants