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

[7.x] Convert eloquent collection to eloquent query builder #33356

Merged
merged 2 commits into from
Jun 29, 2020
Merged

[7.x] Convert eloquent collection to eloquent query builder #33356

merged 2 commits into from
Jun 29, 2020

Conversation

ahmedsayedabdelsalam
Copy link
Contributor

Some times we have a collection of models that we need to update all of them at once

I found the regular approach is to loop on every single model and make (N) times update query

I can build new query from scratch to update them all something like Model::whereIn('id', $items->pluck('id'))->update([])
but I think writing it this way is ugly what if we can convert this collection of models to a query builder looking for those models under the hood.
so we can write it like that $items->toQuery()->update([])

the following image is from a project I am working on it. I am using it as a macro and I thought it could be useful if added to the framework.

Screen Shot 2020-06-27 at 12 14 32 AM

@ahmedsayedabdelsalam ahmedsayedabdelsalam changed the title convert eloquent collection to eloquent query builder [7.x] convert eloquent collection to eloquent query builder Jun 26, 2020
@GrahamCampbell
Copy link
Member

This won't work in general. If there are lots of items in the collection, you will bust the maximum query length limit of any database.

throw new LogicException('can not get Eloquent QueryBuilder from an empty Collection.');
}

return $model->newModelQuery()->whereKey($this->modelKeys());
Copy link
Member

Choose a reason for hiding this comment

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

I assume you wanted "where in" and not "where". Have you tried this code for real?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2020-06-27 at 1 04 03 AM

yes I tried it and loadCount function is implemented the same way

@ahmedsayedabdelsalam
Copy link
Contributor Author

ahmedsayedabdelsalam commented Jun 26, 2020

This won't work in general. If there are lots of items in the collection, you will bust the maximum query length limit of any database.

is Model::update([]); will not work too ?

This PR has nothing to do with the update method it's just querying the models with ids so I think if there is bug in the implementation it would be in the already exist methods but loadCount method uses the same concept.

@GrahamCampbell GrahamCampbell changed the title [7.x] convert eloquent collection to eloquent query builder [7.x] Convert eloquent collection to eloquent query builder Jun 26, 2020
@taylorotwell taylorotwell merged commit 96d9ac5 into laravel:7.x Jun 29, 2020
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