-
Notifications
You must be signed in to change notification settings - Fork 11k
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
Eloquent eager loading with limits #4835
Comments
Is it related to #4841? Try to see what actual generated sql is by calling ->getQuery()->toSql() instead of |
Yeah the limit is applied once. |
If anyone stumbles upon this issue, the following approach seems to work: http://softonsofa.com/tweaking-eloquent-relations-how-to-get-n-related-models-per-parent/ I'm not exactly sure how it works, and it looks a little freaky, but it seems to work. |
still facing this in laravel 5.4 and there seems to be no easy wat to do this, based on how eloquent handles its queries today. @Blackthorn42 any solution ? @taylorotwell the problem here is, it will limit once, as you said, and thus : if i want to get 10 posts with 3 comment each, here i'll only get 3 comments for the whole query and not 3 comments per posts. So far the only simple method was to load each comments by entry, giving 10 more queries in my example (10 more if you add the relationship with the comment author). here was a solution way to complicated and more based on DB design than the use of the framework : https://softonsofa.com/tweaking-eloquent-relations-how-to-get-n-related-models-per-parent/ In IRL, if you are paging a feed for example, 20 posts per page, with their author and 5 comments per post with their author too you get 2 reqs (post and post author) + 2 reqs (comment + comment author) x 20 (number of result) = 42 reqs Is there a plan to solve this issue at some point ? Though this is more a performance issue than anything else but also for Eloquent's API to be cleaner, as the following, is really not clear when reading it, or should i say, doesn't act as the code seems to say it should act.
|
i am having the same issue, i thought that was the only one with that problem |
Same issue here. Isn't there a simple solution like a conditional eager load statement? |
This issue should be re-opened. Ping @themsaid. |
This is not a bug, it's a limitation. We're open to PRs that might address that limitation. |
This, among a few other things, just shows that we must treat Eloquent as a tool, not a solution. It's just a wrapper on SQL. And SQL itself never was, in my opinion, the most natural way to express querying data in anything programmed. |
@themsaid This is not a limitation after debugging and R&D I got the solution. first append the relation count key Model Then add relation
define relation with the limit
At last call the final function and you will get the desired result I have also used this solution for nested relationship and it works perfectly |
@raojeet well... this isn't eager loading so your solution doesn't solves the issue. |
But you can use this trick with pagination. |
Though this is not a good way to do this, it works. The idea is to fetch a chunk of result, then trim it down to how many rows you require. In this example, 5 posts for each category. Also the category here is defined. If the category was not defined. You could use a limit (3). Scenario of this example : Tables :
What I am trying to do ?
What I have ?
Output :
|
@977developer performance issue.... and also needs to change a lot of code in the vue components |
I'm running into this today with Laravel 5.4 and I wanted to include my work around for any still encountering this issue. For the original post for this issue, I'd assume that there is a relationship setup similar to this on the ConversationModel. public function messages()
{
return $this->hasMany('App\Message', 'conversation_model_id');
} My workaround, is to include a new scoped relation based on the orignal Here's what that looks like: public function recent_messages()
{
return $this->messages()->orderBy('created_date')->take(5);
} And then, in my query, I am able to do something similar to this. App\ConversationModel::where('user_id', 1)->with('recent_messages')->get(); May not work well in all use cases, but I wanted to include it here in case anyone else runs into this issue. |
UNION ALL solution |
Be careful @collinticer as it will load only 5 messages for all the Users, and those 5 messages will be distributed to their respective User in the results. But it will not retrieve 5 messages for each User if ever each User had 5 or more messages. That's why you will see amongst all Users that has messages, some will has no message while some will have 1 or more messages. |
@KeitelDOG That makes sense and explains why I likely didn't notice that result during development. Thanks for catching that! |
@collinticer , so are you saying that your example does NOT give the expected outcome of 5 messages for each user? If not, have you found a better workaround? This should really be fixed in Laravel as a framework. |
@WallyJ alright so I did some further testing. In a single model use case, my solution works fine. So if you are only ever fetching a user and then outputting his most recent messages, my solution should work. So something like However, if you are doing something where you are fetching a collection of users and trying to include their most recent message using something like If you come up with a solution that works, I'd love to hear about it. I agree that it would be a nice feature to have it addressed directly in the Laravel Framework. |
So, I have logged in users, who have contacts, which have contact notes. I want the 3 most recent notes for each contact, belonging to a given user. This comes out as a collection of contacts for the logged in user. Sounds like your solution won't work for me as I am trying to use take(3) on each member of a collection, which doesn't seem to be a working function within Laravel at the moment. Am I understanding you correctly? |
Yes that's correct. Since you are working with a collection and trying to access a recent notes function for each item in the collection, my work around will not work for you. You could not simply do I think a pretty easy work around for this though would be something like this: foreach($contact in $user->contacts){
$contact->recent_notes()->get();
} In many use cases, I think that little snippet would work fine if you don't mind iterating over the collection on page load like that. |
I released the rejected PR as a package: https://github.com/staudenmeir/eloquent-eager-limit |
If you're using PostgreSQL (and you should), just use the SELECT DISTINCT ON (location) location, time, report
FROM weather_reports
ORDER BY location, time DESC; Eager load the last message for each conversation: $conversations = $user->conversations()
->with(['messages' => function ($query) {
$query->selectRaw('DISTINCT ON (conversation_id) conversation_id, *');
$query->orderByRaw('conversation_id, created_at DESC');
}])
->get(); |
just do like this :
replace your model on before query. |
Eager loading with limit will be supported natively in Laravel 11 (#49695). |
We are trying to implement eager loading with eloquent, where we want to get 5 conversations and for each conversation the 10 most recent messages. It seems that Eloquent only applies the limit once, returning 10 messages in total (as opposed to the expected result of up to 10 messages per each of 5 conversations).
The code:
The problem seems to be the
$query->take(10);
part, which seems to apply only "once" on the whole query.We tried to google on possible causes of this, but while it seems that quite a few people have the same problem, there is no good solution - with the exceptions of workarounds, like fetching the messages programatically, which are not really ellegant.
Is this a bug or is this intended behavior?
The text was updated successfully, but these errors were encountered: