Pagination does not work when use having in query builder

[andreipasat]

• Laravel Version: 5.3.9
• PHP Version: 7.0.8
• Database Driver & Version:5.7.12

Description:

The problem appears when get some results from database using query builder. If apply first having method and after paginate will generate sql error that variable used in having clause is not in select. paginate() method have second param as an array of columns for select, but output sql is have error because generate something like select count(,my_having_variable). Ex : `$distance = DB::raw('( 6371 ACOS( COS( RADIANS('.$post['lat'].') ) COS( RADIANS( lat ) ) COS( RADIANS( lng ) - RADIANS('.$post['lng'].') ) + SIN( RADIANS('.$post['lat'].') ) * SIN( RADIANS( lat ) ) ) ) as distance');`

$profiles = DB::table('profiles')->select('profiles.*','profiles.id as profile_id', $distance); $dist = 20; $profiles->having('distance', '<=', $dist); $results = $profiles->paginate(8); // like that i get error that "distance" is not defined becausepaginate select just count(*)`

I need use :

$results = $profiles->paginate(8,['*',$distance]); for not have this error but i get another error because it's generating a sql like that :

select count(*,( 6371 * ACOS( COS( RADIANS ..... as distance) as aggregate and not select count(*) as aggregate, ( 6371 * ACOS( COS( RADIANS ..... as distance how I think should be

The problem is not correct interpretation in function compileAggregate from Grammar.php.

I modify there like that :

    `$sql = 'select ';       
      $column = $aggregate['columns'][0];

    $addSelect = '';
    if (count($aggregate['columns']) > 1) {
        for ($i = 1; $i < count($aggregate['columns']); $i++) {
            $addSelect .= ',' .$aggregate['columns'][$i];
        }
    }

    $sql .= $aggregate['function'].'('.$column.') as aggregate' . $addSelect;

    return $sql;`

And before call paginate need to add DB::unprepared("SET sql_mode = ''"); if you have having. I know is not universal solution because is not even taken in consideration if use distinct but for me solves the problem.

Steps To Reproduce:

https://github.com/laravel/framework/issues/15675

[n7olkachev]

Also refs #14474

[themsaid]

Wouldn't that query error in strict mode?

select count(*) as aggregate, ( 6371 * ACOS( COS( RADIANS ..... as distance

Something like:

In aggregated query without GROUP BY, expression #2 of SELECT list contains nonaggregated column '*****'; this is incompatible with sql_mode=only_full_group_by

[andreipasat]

@themsaid Thats why need to execute DB::unprepared("SET sql_mode = ''"); before paginate().

[themsaid]

But it doesn't feel right to turn a mode on/off during runtime, either your application is on strict mode or not, laravel by default has the strict mode on, and in this case such query won't work.

I think it's better that you build the paginator yourself because I don't think laravel will be able to handle such query, the paginator also has limitations with using distinct, but that's something that can be solved, however for having I really don't think there's an easy way to make it work.

[themsaid]

I think for this specific case it'd be better if you run the queries and build the paginator manually since this case require special mode handeling.

[andregaldino]

@andreipasat I made pagination manually.

Controller

$establishments = Establishment::Distance($geoip->lat,$geoip->lon)->get();
$currentPageSearchResults = $establishments->slice(($currentPage - 1) * $perPage, $perPage)->all();
$establishments = new LengthAwarePaginator($currentPageSearchResults, count($establishments), $perPage);

and View, after foreach

{!! $establishments->appends(Input::except('page'))->render() !!}

but, i didn't like of the performance, because i need get all records for after make pagination. anyway, it works, I post here when improve performance.

[AkenRoberts]

@andregaldino You can improve the performance by making the first query a ->count() to get the total, and then query again using skip() and take() to only retrieve the appropriate slice of records.

[nisbeti]

@themsaid Do you have a work-around for the distinct problem that you mentioned above https://github.com/laravel/framework/issues/16320#issuecomment-259704843 ?

[andregaldino]

Yeah, it's better do it manually like that, without LengthAwarePaginator. Thanks for sharing @cryode

[ahmedash95]

Any solution instead of doing manual pagination ?

[n7olkachev]

Nope, Query Builder is just not smart enough to do count query with having statements.

[ahmedash95]

I did it by custom pagination and it works fine now

[isometriq]

What about this form?

select count(*) as aggregate from (
select concat(contacts.first_name,' ',contacts.last_name) as name from contacts having name like '%joe%'
) AS count;

[n7olkachev]

@isometriq yep, it should work like this.

[isometriq]

So to do it, I would:

1. generate the sql from my query builder object
2. append the sql to from of an new aggregate query
3. execute aggregate query and get the count
4. execute query builder and get row objects
5. create pagination object from both query results

Not sure how to do this the eloquent/query builder way or when to leave their boundaries.

[n7olkachev]

Builder::macro('fromQuery', function ($query) {
    $sql = $query->toSql();
    $bindings = $query->getBindings();

    return $this->from(\DB::raw('(' . $sql . ') as t'))->setBindings($bindings);
});

...

$productsCount = \DB::query()->fromQuery($productsQuery)->count();

This is how I've dove it. $productsQuery is Eloquent\Builder.

[isometriq]

Very nice, thanks

[progmars]

I find that the same issue is present even without having:

 $paginator = $this->query->paginate(10, ["name", "otherField"]);

and on SQLite it generates a query:

 select count("name", "otherField") as aggregate from "test_entries"

and SQLite fails with SQLSTATE[HY000]: General error: 1 wrong number of arguments to function count()

The culprit seems to be in Builder class:

public function getCountForPagination($columns = ['*'])
{
    $this->backupFieldsForCount();

    $this->aggregate = ['function' => 'count', 'columns' => $this->clearSelectAliases($columns)]; <--- why count on all columns?

    $results = $this->get();

So, yeah, the $columns option in paginate() is misleading, currently it cannot be used at all unless selecting just one column.

In Rails they have special workaround - to reset columns for count() to :all for the same case:

https://github.com/rails/rails/issues/14630

[royduin]

Just use this package which fixed it: https://github.com/justbetter/laravel-pagination-with-havings