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

[5.2] Lazy Eager Loading issue in LengthAwarePaginator collection #14468

Closed
vigneshgurusamy opened this issue Jul 25, 2016 · 10 comments
Closed

Comments

@vigneshgurusamy
Copy link
Contributor

After upgrading to Laravel 5.2.41, I'm getting Method load does not exist exception while trying to call load() method in LengthAwarePaginator collection.

I found out Eloquent Builder.php paginate() method sets empty array if total count of the result set is zero. So LengthAwarePaginator converts the empty array into \Illuminate\Support\Collection which does not have load() method.

public function paginate($perPage = null, $columns = ['*'], $pageName = 'page', $page = null)
    {
        $page = $page ?: Paginator::resolveCurrentPage($pageName);

        $perPage = $perPage ?: $this->model->getPerPage();

        $query = $this->toBase();

        $total = $query->getCountForPagination();

        $results = $total ? $this->forPage($page, $perPage)->get($columns) : [];

        return new LengthAwarePaginator($results, $total, $perPage, $page, [
            'path' => Paginator::resolveCurrentPath(),
            'pageName' => $pageName,
        ]);
    }

Kindly return Eloquent empty collection instead of an empty array, so eager loading won't fail.

@srmklive
Copy link
Contributor

Can you give a code example for it?

@vigneshgurusamy
Copy link
Contributor Author

@srmklive

To explain this scenario, I'm going to use posts and comments relationship.
For example, I have to fetch first 10 posts, but there are no posts in the table.

$posts = Post::paginate(10);

In Eloquent builder paginate() method,

$total = $query->getCountForPagination();

$results = $total ? $this->forPage($page, $perPage)->get($columns) : [];

In this scenario, $total will be zero and $results will be set as an empty array instead of Eloquent collection.

In LengthAwarePaginater, this empty array got converted into support collection.

My code to load comments for these posts using load() method is like below:

$posts->load('comments');

Now load() fails because the given collection is Support collection not an Eloquent collection.

@srmklive
Copy link
Contributor

Have you done it like this?

$posts = Post::with('comments')->paginate(10);  

What results does the above line gives you?

@vigneshgurusamy
Copy link
Contributor Author

@srmklive I know with() works to load relations. But in some cases, I have to load relations after obtaining the results

@srmklive
Copy link
Contributor

My suggestion would be that you utilize with to load relations. You can simply if the related data is empty or not. This is a better approach in my opinion:

@if (!empty($post->comments))

@endif

@vigneshgurusamy
Copy link
Contributor Author

@srmklive By using your way, it defeats the purpose of Eager loading.

@jlem
Copy link

jlem commented Jul 25, 2016

@vigneshgurusamy without taking a closer look at the code, I tend to agree with you that it should return an Eloquent collection, not a Base collection. That said, you can always wrap the results in an Eloquent collection yourself.

You can just do $results = new Illuminate\Database\Eloquent\Collection($posts). This will convert it to an Eloquent collection whether it happens to be an array, or another collection.

It's a bit of a manual work-around, but at least it will get the job done.

@vigneshgurusamy
Copy link
Contributor Author

vigneshgurusamy commented Jul 25, 2016

@themsaid I'm getting this issue after your pull request #14389 merged (v5.2.41). This issue was not there until v5.2.39.

Can you take a look?

@vigneshgurusamy
Copy link
Contributor Author

@jlem Thanks for the support.

Am I afraid, I can not do that since I need the results as LengthAwarePaginator object with Eloquent collection as a paginator collection.

@themsaid
Copy link
Member

Well the actual change occured in this PR: #14188

Mine was to fix some issue with it but it looks like the original PR has caused another issue, will submit a fix in a minute.

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

No branches or pull requests

4 participants