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

[6.x] Added array to json conversion for sqlite #30133

Merged
merged 2 commits into from
Oct 4, 2019

Conversation

i-bajrai
Copy link
Contributor

In reference to #30128, this PR allows SQLite databases to cast arrays as JSON when calling the update method directly on a model.

This allows a more consistent result when testing between mysql and sqlite, especially when using the $casts array on a model.

@i-bajrai
Copy link
Contributor Author

@staudenmeir I hope this resolves the issue!

@driesvints driesvints changed the title Added array to json conversion for sqlite [6.x] Added array to json conversion for sqlite Sep 30, 2019
@@ -193,6 +193,12 @@ protected function compileUpdateWithJoinsOrLimit(Builder $query, array $values)
*/
public function prepareBindingsForUpdate(array $bindings, array $values)
{
$values = collect($values)->reject(function ($value, $column) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the reject() clause. It's not relevant until we support JSON UPDATE queries on SQLite (i.e. ->update(['foo->bar' => 'baz'])).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, thanks for that, I wasn't sure why it was there, the postgres grammer was built differently so a comparison was hard.

@taylorotwell taylorotwell merged commit 9a81e30 into laravel:6.x Oct 4, 2019
i-bajrai added a commit to i-bajrai/framework that referenced this pull request Oct 4, 2019
* Added array to json conversion for sqlite

* Removed reject from collection, not relevant unless using JSON queries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants