Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Error when updating multiple rows in same group #210

Closed
AhmedAli-Qawafel opened this issue Mar 27, 2023 · 3 comments
Closed

Error when updating multiple rows in same group #210

AhmedAli-Qawafel opened this issue Mar 27, 2023 · 3 comments

Comments

@AhmedAli-Qawafel
Copy link
Contributor

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.

@AhmedAli-Qawafel
Copy link
Contributor Author

[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}

@AhmedAli-Qawafel
Copy link
Contributor Author

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;
    }

@rubenvanassche
Copy link
Member

Could you send in a PR adding this? I think if all tests are green this one can be merged into the package.

@spatie spatie locked and limited conversation to collaborators Mar 30, 2023
@rubenvanassche rubenvanassche converted this issue into discussion #212 Mar 30, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants