Skip to content

Only update changed fields in save()#37

Open
Crotalus wants to merge 10 commits into
zombor:masterfrom
Crotalus:master
Open

Only update changed fields in save()#37
Crotalus wants to merge 10 commits into
zombor:masterfrom
Crotalus:master

Conversation

@Crotalus

@Crotalus Crotalus commented May 9, 2011

Copy link
Copy Markdown

Keep track of changed fields and only UPDATE those when doing save()

ID is also updateable with this patch, maybe it's not desireable?

@zombor

zombor commented May 9, 2011

Copy link
Copy Markdown
Owner

Why bother with this? I don't see the advantage vs. the extra code required to support it.

@Crotalus

Crotalus commented May 9, 2011

Copy link
Copy Markdown
Author

Why send extra data over to the database if it isn't necessary? The DB is usally the bottleneck anyway, not webfronts.

PostgreSQL has optimizations that makes an UPDATE in-place unless the UPDATE-query touches a field with an index on it, then it needs to create a whole new row in the table, resulting in the need for VACUUM/REINDEX etc.

Another usecase would be if you have a big field in your model, say a field storing some binary encoded data. To send it over to the database when you're just increasing a counter seems unnecessary.

What if you have some triggers on the DB-side on specific field-updates, they would trigger all the time although not really changed.

The DB-logs will also be cleaner, easier to see exactly what rows being updated.

@Crotalus

Crotalus commented May 9, 2011

Copy link
Copy Markdown
Author

Another reason:

Less risk of overwriting data if two processes updates the row right after eachother, the second process could have "stale" unchanged fields that shouldn't be stored in the DB.

@Crotalus

Crotalus commented Jun 3, 2011

Copy link
Copy Markdown
Author

Closing pull request

@Crotalus Crotalus closed this Jun 3, 2011
@zombor zombor reopened this Jun 3, 2011
@zombor

zombor commented Jun 3, 2011

Copy link
Copy Markdown
Owner

Why? I'm still considering this feature. It seems like a good idea.

@Crotalus

Copy link
Copy Markdown
Author

I closed it due to me being sloppy and committing some stuff to master instead of a specific feature-branch polluting this request :)

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

Successfully merging this pull request may close these issues.

2 participants