Can We Integrate Save The Change into Django's Core? #1

saulshanabrook opened this Issue Sep 9, 2013 · 3 comments

3 participants


Is there anything stopping this feature from being merged into core Django?

Has there been any discussion or tickets created to add this feature?


Djagno ticket #4102 discusses adding a feature like this, but was changed into adding the update_fields argument to save instead. I am going to mention this project as well on that ticket, but wasn't sure if there is some reason that this shouldn't be brought up as a an issue.


I think there are a number of reasons perhaps not to add this to Django proper. They're not necessarily all valid, but here's how I see it:

1) This'd be a sharp departure from the type of flow developers are used to. This is less of an issue for experienced developers who are doing things properly, but there are surely a class of developer who rely on a save() overwriting all the columns. Of course, the same reasons they rely on that behavior are the reasons they should be enforcing read/write locks or other methods to ensure data integrity. But at least in the current model the entire row remains internally consistent.

The counterargument here is that there'd be a group who prefer this more granular update by default. If, for example, two users update the same news article at the same time, one adjusting the slug, and the other changing some of the body text, this granular save would have a preferable effect: both improvements to the article would be made; one wouldn't block out the other.

2) Developers can already do this by using something like this by mixing in a class, the reverse is perhaps clunkier, in so far as having a kwarg that forces non-granularity seems non idiomatic. I don't really have a counterargument for that.

3) There's a trade off in performance. We're trading in a little bit of memory and CPU to send less data down the wire to the database and make marginally faster writes. For me, that's the right trade off: I've got memory and CPU to spare, but I'm making tons of writes and my database certainly feels it. But it's not perhaps the nicest thing to add yet another bit of memory consumption to Django.

Of course, I've done everything possible to minimize memory usage: _changed_fields is empty when all fields have their original database values, and if any field is reset back to its database representation, it's excised from the dict. But still, it's taking up a little bit more memory and a little bit more CPU, there's no getting around that.

And honestly, this might make the overall operation slower. It certainly makes the actual database write faster, and that's awesome if you're me, but it's not necessarily what everyone else wants (a slower overall save for a faster db write).

4) There's no way to make this behavior standard across reverse relational and ManyToMany fields without a bit of an overhaul of Django's exceptional handling of those cases. As I understand it, currently changed made to these fields are committed immediately. This isn't necessarily in line with how save() is understood to work, but if you're expecting functionality like that in the included TrackChanged mixin, you're going to be really disappointed when you find you can't easily do things like rollback the changed values on those fields.

Then again, I didn't spend much time trying to get it to work, I just assumed that it wasn't possible as I was told as much. Perhaps it's completely doable with a little bit (but not a ton) of work.

5) There may be a whole bunch of reasons that I'm not clever enough to come up with for why this is a bad idea in enough cases to warrant it not being the default. This is basically just a catch-all: I'm not sure what other arguments there may be against this.

6) I use tabs instead of spaces. This turns out to infuriate people.

@karanlyons karanlyons closed this Sep 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment