feat(entity): optimize entity save method and add bulk save functionality#663
feat(entity): optimize entity save method and add bulk save functionality#663serbanPricop wants to merge 5 commits intojazzband:masterfrom
Conversation
…lity - Add `commit` parameter to `Entity.save()` method - Implement `save_bulk()` method for efficient bulk creation of EAV values - Modify `save()` method to collect values and use `save_bulk()` when commit is True - Improve performance by reducing database queries during save operations
for more information, see https://pre-commit.ci
- Add `commit` parameter to `save` method - Implement `save_bulk` method for preparing bulk EAV Value objects - Modify `save` method to support bulk creation when `commit=False` - Update attribute value handling in `save` method
for more information, see https://pre-commit.ci
- Change `save` method signature to use keyword-only argument
|
Thanks for the PR @serbanPricop! Looking at the use cases for this, I see two use cases I'll keep in mind when adding some comments on the PR. Please do correct me so I can be sure I align with your thoughts! The use cases for this PR I see are:
Looking at
Being able to |
| if not commit: | ||
| attributes_value = self.save_bulk(values, attributes) | ||
| Value.objects.bulk_create(attributes_value) |
There was a problem hiding this comment.
If commit=False, I would expect nothing to be saved to the database, which is line with Django behavior. Instead, what do you think about returning a list[Value] back? This would work for any number of attributes that are updated during an eav.save() call.
| def save_bulk( | ||
| self, | ||
| eav_values, | ||
| attributes, | ||
| ): |
There was a problem hiding this comment.
In line with the other comment, should bulk_save() take a list[Entity]? Or is the use case for this something different than the two scenarios I identified earlier?
Thinking of how Django's bulks_create() works, you pass in a list of objects created in memory that then get written with an optimized database query. In this scenario, I would think that when wanting to update a number of entities, you would do something in your app like:
for patient in patients:
patient.eav.age = 10
patient.eav.height = 2.3
eav.bulk_save(patients, 10)
Patient.objects.bulk_create(patients)
Note I did a generic eav.bulk_save() as I'm not sure this should be on the Entity manager as inserted into a specific Entity instance versus being on a Model Manager. Maybe that's a future thing, having a manager for a model that has EAV methods on it, but for now, I think a utility function would work just fine too.
Also, thinking about it, we should use the same language as Django with "bulk_update" and their params to not have confusion around things like expecting signals to be called like a typical save()
What do you think?
commitparameter toEntity.save()methodsave_bulk()method for efficient bulk creation of EAV valuessave()method to collect values and usesave_bulk()when commit is TrueI'm helping!
Checklist
CHANGELOG.mdPull Request type
Please check the type of change your PR introduces:
Related issue(s)
Other Information