yaddly's avatar

orderBy method not working on Eloquent Builder

I'm trying to order my result set using eloquent builder but it is not working. I tried investigating this unfortunate phenomena but I only learnt that according to the api documentation accessible : here

The eloquent builder class does not have orderBy() method. Am I wrong to say Eloquent Builder is a wrapper for Query Builder? If so, why is orderBy() method missing from Eloquent Builder class but does not throw call to undefined method error when used and why is it not working as expected? To access orderBy('sortColumn') method, I call getQuery() on the Eloquent\Builder instance which returns an instance of Query\Builder.

use Illuminate\Support\Str;
use Illuminate\Http\Request;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection as EloquentCollection;

private static function getResults(Builder $builder): EloquentCollection {

        /* return new EloquentCollection($builder?->getQuery()->orderBy(request()?->filled('sortColumn')

            ? request('sortColumn')

            : 'movieTitle')?->get()); */



        return $builder?->when(request()?->filled('sortColumn'), function ($builder) {

            return $builder

                ?->getQuery()

                ?->orderBy(Str::squish(request('sortColumn')));

        }, fn ($builder) => $builder)?->get();

    }

The code above throws the exception below when sorting is applied, but works fine if sorting is not applied.

Error: Return value must be of type Illuminate\Database\Eloquent\Collection, Illuminate\Support\Collection returned.

I tried again to convert the Support\Collection back to Eloquent\Collection using commented code in the code sample above, but this too, threw an exception below when I attempted to paginate the result set using the code below:

Error: Call to undefined method stdClass::newModelQuery()

$resultSet?->toQuery()?->paginate(8);

I cannot directly paginate Eloquent\Collection, so I call toQuery() method to access the Eloquent query builder from the collection and use it to access the paginate(8) method.

I would greatly appreciate any help you can afford me in getting the Eloquent\Builder to sort data using orderBy('sortColumn') method and return an Eloquent\Collection that can be paginated.

0 likes
7 replies
JussiMannisto's avatar

You can use orderBy with the Eloquent builder so you shouldn't call getQuery(). All query builder methods are available to the Eloquent builder through forwarded calls. I won't go into the details here.

You're using the sortColumn input as the column name without any visible validation checks. This allows SQL injection attacks. If you've validated the column name elsewhere, you should pass it to the method as a parameter. Using the global request() helper here without validation is pretty dangerous.

1 like
yaddly's avatar

@JussiMannisto Hi, thank you for your kind response. Unfortunately, I did call orderBy() method directly on the eloquent builder instance but it didn't work. It only worked through Query\Builder instance hence a call to getQuery() method. Thank you pointing out the need for validation on sortColumn, logic for this is contained within the FormRequest. I had to simplify things for brevity.

JussiMannisto's avatar

@yaddly Can you show the code where you're calling orderBy() and it doesn't work? Because that's a really common thing to do. Something else must be wrong there.

JussiMannisto's avatar

@yaddly

I cannot directly paginate Eloquent\Collection, so I call toQuery() method to access the Eloquent query builder from the collection and use it to access the paginate(8) method.

This makes no sense. You would load the entire result set from the DB, build a new query based on that, and then get a single page of results from the DB.

All you need to do is is call paginate() on the query builder the first time.

1 like
yaddly's avatar

@JussiMannisto Hi, this code is the last piece of data filtering logic. There are multiple optional filters being applied elsewhere. Once that is done, the last action is sorting by a dynamic column from the request and return the results. This explains why the method returns a collection.

JussiMannisto's avatar

@yaddly You're not filtering data, you're building a database query. The query is performed at the point where you call get() or paginate(). The first one retrieves the entire result set, the second retrieves a single page of results (wrapped in a paginator object).

The point of pagination is to avoid retrieving the whole result set at once, which is horrible for performance and might crash your app due to memory exhaustion if the table is large enough.

The method you described would 1) read the entire result set into memory 2) make an unnecessary second DB query that retrieved a subset of that same data. Unless you need the entire result set on every request, you should paginate on the 1st query and not do a 2nd query at all.

1 like
yaddly's avatar
yaddly
OP
Best Answer
Level 2

@JussiMannisto Hi, thank you for your pointers. I modified the code as shown below and it worked fine.

return $builder?->when(request()?->filled('sortColumn'), function ($builder) {
            return $builder
                ?->orderBy(Str::squish(request('sortColumn')));
        }, fn ($builder) => $builder)?->paginate(8);

Please or to participate in this conversation.