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

update should only update changed fields on record #472

Open
kalinon opened this issue Apr 25, 2023 · 4 comments
Open

update should only update changed fields on record #472

kalinon opened this issue Apr 25, 2023 · 4 comments

Comments

@kalinon
Copy link
Contributor

kalinon commented Apr 25, 2023

Currently Granite will perform an UPDATE and provide all fields for the update statement. It should only perform an update on the fields that changed for optimization.

@crimson-knight
Copy link
Member

Doing this means we need something akin to the dirty module provided by ActiveRecord.

It's a good idea, I'm all for it.

In AR it's not always clear if this is automatically enabled or not. I think this should come standard enabled so no one has to think hard about it.

Link to the module so anyone else can check and get an idea of what's going on.
https://api.rubyonrails.org/classes/ActiveModel/Dirty.html#method-i-changed-3F

This means we need to add:

  1. In memory tracking of the original object values and their types
  2. Update/override the setter methods so that it can track when an attribute value has changed, and which attribute it is, prior to assigning the new value

My first thought is that we should probably do this with a hash that takes all values in as String for key and value, but can convert them back to the column type when doing any comparison of the old and new value. I could be wrong on this, but, from my understanding it's Union types that increase compile time and potentially are a performance hit, so having a single type and just converting on the fly would be more performant.

That part is probably worth testing to confirm.

@kalinon
Copy link
Contributor Author

kalinon commented Apr 25, 2023

We could possibly use the hashdiff lib i wrote: https://github.com/spoved/hashdiff.cr

tho, it would require keeping a "pristine" record and "dirty" record to compare. which i dont know if i like. probably some sort of dirty flag and a record of which field was changed would be a better path.

@crimson-knight
Copy link
Member

@kalinon I'd rather not introduce outside libraries into Granite if at all possible, and I think this is simple enough that including it in Granite itself should be easy to do.

@a-alhusaini
Copy link
Contributor

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

No branches or pull requests

3 participants