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

Curious on why model.save(only=model.dirty_fields) isn't default behavior #643

Closed
ibushong opened this issue Jul 2, 2015 · 7 comments
Closed

Comments

@ibushong
Copy link

ibushong commented Jul 2, 2015

This definitely isn't an issue, I was just curious on this design decision.

Why isn't the default behavior to only update modified fields? (i.e. the behavior we get when using model.save(only=model.dirty_fields)

When viewing SQL logs, it's much easier to see what's being updated. If I update a single field, then the UPDATE query reflects that. But if all fields are persisted then it's more difficult to see what changed. Though as a counterpoint. it's easer to see what the exact value of a row is after any given UPDATE query if we explicitly re-set all fields.

Also, for extremely large rows, you'd be transferring less data if only updating modified fields.

Obviously it's easy to implement this behavior, but just wondering why it's not the default case.

Thanks! peewee is excellent

@coleifer
Copy link
Owner

coleifer commented Jul 2, 2015

Peewee is active record, the behavior is Django-like, so save() will save everything by default. Changing that now would probably break existing implementations anyways.

You can always subclass Model and add your own save() implementation that does what you want.

But to answer your question, mostly it is this way because that is how Django's ORM behaves and I originally based peewee off of that. I can't change it now because I'd worry about breaking people's code.

@coleifer coleifer closed this as completed Jul 2, 2015
@ibushong
Copy link
Author

ibushong commented Jul 2, 2015

Yeah I wasn't suggesting it be changed, just curious on why. Thanks for the explanation! That clears it up.

@shon
Copy link

shon commented Oct 15, 2015

I am bitten by this a couple of times as well. I use peewee in production.

@coleifer Peewee's API for most cases very obvious and simple and perhaps that's why it's loved by so many (including me). But I feel that "because django orm does thing certain way" isn't a very good explanation. Peewee is peewee and it should do what's best and more obvious for users.

Having said that this may break existing code probably a valid argument (still not 100% convinced that it will break). But I wonder is it possible to have some setting like

peewee.on_save_always_use_dirty_fields = T/F  # default being False

I think most of us would not expect below code to wipe out data in non selected columns

class Profile(Model):
    """
    has lot of fields because of various reasons
    """

a_profile = Profile.select(Profile.id, Profile.registered).where(..).first()
a_profile.registered = True
# You have a cleaner profile now :)

@ibushong
Copy link
Author

@shon, the reason is looks like your Profile object has been "cleared" is because you're only selecting the id and registered fields. Remove the arguments to select() and you should see the behavior I think you're expecting. In general, you shouldn't really need to select specific fields. It's just gonna make your code harder to maintain.

@coleifer
Copy link
Owner

Just pushed 376a467, which adds a new Meta option: only_save_dirty (default is False). You can now mark you profile:

class Profile(Model):
    fields = WhateverFields()

    class Meta:
        only_save_dirty = True

When you call Model.create() all fields will stll be saved, it is only when you call .save() by itself. You can explicitly save non-dirty fields by specifying them using .save(only=[whatever]).

@coleifer
Copy link
Owner

Thanks for the suggestion, @shon, I like options.

@shon
Copy link

shon commented Oct 16, 2015

Awesome. Thank you so much @coleifer

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

3 participants