Error when updating multiple rows in same group
AhmedAli-Qawafel opened this issue · 3 comments
Description:
When trying to update multiple rows which belong to the same group, our application is saving each value alone to the database, using a collection each method and looping through each property to update it individually. This is resulting in a significant performance hit, as it is generating multiple queries to the database, instead of updating all rows in a single query.
We have considered using the update method provided by Laravel's Query Builder or Eloquent ORM to update multiple rows in a single query, but we are encountering errors when attempting to do so.
Specifically, we are seeing errors related to mismatched data types or SQL syntax errors when attempting to use the whereIn method to specify the IDs of the rows to be updated, and then chain the update method to set the new values for the relevant columns.
We believe that this issue may be related to the structure or configuration of our database, or to the way that our models are defined. However, we are not sure how to diagnose or resolve the issue.
We would appreciate any guidance or assistance that can be provided to help us resolve this issue and improve the performance of our application.
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 1000, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.24}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 0, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.17}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 2, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.77}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 'true', "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.5}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 0, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.48}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 2, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.19}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 'false', "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.54}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 0, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.49}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 2, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.3}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 2000, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.14}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 1000, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.26}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 64, "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.47}
[2023-03-27 19:24:39] local.DEBUG: update "settings" set "payload" = 'true', "updated_at" = '2023-03-27 19:24:39' where "group" = 'anyGroup' and "-------" = '-------' {"time":3.45}
I did a solution for this I would love to get your opinion on it.
So I overrode your save
method to use the upsert
method instead of updating each row individually. This makes the code more efficient and reduces the number of queries sent to the database. Let me know what you think! Thanks!
public function save(): self
{
$properties = $this->toCollection();
event(new SavingSettings($properties, $this->originalValues, $this));
$config = app(SettingsMapper::class)->initialize(static::class);
$updates = [];
$changedProperties = $properties
->reject(fn ($payload, string $name) => $config->isLocked($name))
->each(function ($payload, string $name) use ($config, &$updates) {
if ($cast = $config->getCast($name)) {
$payload = $cast->set($payload);
}
if ($config->isEncrypted($name)) {
$payload = Crypto::encrypt($payload);
}
// Remove the updatePropertyPayload call and save the updates in an array
$updates[] = [
'group' => static::group(),
'name' => $name,
'payload' => json_encode($payload),
];
});
event(new SettingsSaved($this));
// Only update the records if there are changes
if ( ! empty($updates)) {
$config
->getRepository()
->getBuilder()
->upsert($updates, ['group', 'name'], ['payload']);
}
$values = app(SettingsMapper::class)
->fetchProperties(static::class, $config->getLocked())
->merge($changedProperties);
$this->fill($values);
$this->originalValues = $values;
event(new SettingsSaved($this));
return $this;
}
Could you send in a PR adding this? I think if all tests are green this one can be merged into the package.