[11.x] Fix incorrect bindings in DB::update when using a collection as a value #53254
Conversation
…s a value fixes #53226 Signed-off-by: Mior Muhammad Zaki <[email protected]>
Signed-off-by: Mior Muhammad Zaki <[email protected]>
| @@ -3848,7 +3848,11 @@ public function update(array $values) | |||
|
|
|||
| $values = collect($values)->map(function ($value) { | |||
| if (! $value instanceof Builder) { | |||
| return ['value' => $value, 'bindings' => $value]; | |||
| return ['value' => $value, 'bindings' => match (true) { | |||
| $value instanceof Collection => $value->toArray(), | |||
There was a problem hiding this comment.
Why not use $value->all() and skip the implicit array_map call inside to Collection@toArray()?
There was a problem hiding this comment.
Also, I'd change the check to $value instanceof Enumerable, so $value->all() also covers LazyCollection instances
There was a problem hiding this comment.
It might not be even worth it to pursue these changes. Using Collection as value is not in the docs (unless someone can share the URL) and Postgres/SQLServer would require different syntax to store JSON
There was a problem hiding this comment.
You're right. And it is easy enough for someone to just add collect(...)->all() on the call-site
There was a problem hiding this comment.
I think it's problematic though that the behaviour is somewhat undefined when passing in a collection (or at least very unexpected), especially as I think many devs don't actively differentiate between arrays and collections in Laravel as much Laravel functionality behaves the same no matter what you pass in.
fixes #53226