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

[11.x] Changes in #53948 cause upsert to fail #54145

Closed
kohenkatz opened this issue Jan 10, 2025 · 2 comments · Fixed by #54169
Closed

[11.x] Changes in #53948 cause upsert to fail #54145

kohenkatz opened this issue Jan 10, 2025 · 2 comments · Fixed by #54169
Labels

Comments

@kohenkatz
Copy link
Contributor

Laravel Version

11.37.0

PHP Version

8.3.13

Database Driver & Version

PostgreSQL 17

Description

When providing an array with non-numeric keys to the Model::upsert method, the behavior in 11.36.0 and older was to use the values of the array as the data. After the changes in #53948, the keys of the array are used as the data, and a database error is returned (because there are no such column names, or because the number of keys does not match the number of values provided).

I understand the rationale for #53948, and I think it makes sense to keep that behavior for Model::insert, but the same behavior makes no sense for Model::upsert because the documentation explicitly describes upsert's first argument as an array of records, not a single record, so using the values makes more sense.

Steps To Reproduce

The following code works in 11.36.0, but crashes with a database error in 11.37.0:

(This is using Laravel's default users table to make it as simple as possible and not need to give a migration as part of the example.)

$users = [
    'user-a' => ['name' => 'User A', 'email': '[email protected]', 'password' => Hash::make('qwerty')],
    'user-b' => ['name' => 'User B', 'email': '[email protected]', 'password' => Hash::make('asdfgh')],
];

User::upsert($users, ['email'], ['name', 'password']);
@sov-cpoerschke
Copy link

We encounter also problems with the changes made in the compileInsert() method as described above. Our unit tests are failing, if the records are no lists. Adding the spread operator in line 1174 of https://github.com/laravel/framework/blob/v11.37.0/src/Illuminate/Database/Query/Grammars/Grammar.php would fix the problem.

? [$values]

should be

? [...$values]

kind regards

This comment was marked as off-topic.

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

Successfully merging a pull request may close this issue.

3 participants