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

withCount relation with Where results in surplus bindings in the count query #1600

Closed
kronthto opened this issue Jan 31, 2018 · 3 comments
Closed

Comments

@kronthto
Copy link

kronthto commented Jan 31, 2018

Summary of problem or feature request

When using withCount with a relation that has own bindings (Wheres), the filtered count query bindings do not match the query.

This can result in recordsFiltered in the response being off or often just zero.

Using the datatables frontend JS, the result count is wrong (Showing 0 to 0 of 0 entries (filtered from 30,280 total entries)), which subsequently causes pagination buttons to disappear. The actual data however is shown and ordering also works.

May be related to #1471 however search/ordering is working in my case.

Code snippet of problem

datatables call:

datatables()->of(User::query()->withCount('activeTokens'))

relation on User:

public function activeTokens()
{
  return $this->tokens()->where('revoked', '=', false)->where('expires_at', '>', new Carbon());
}

When using the globalSearch the following query is executed + bindings:

select `users`.*, (select count(*) from `oauth_access_tokens` where `users`.`id` = `oauth_access_tokens`.`user_id` and `revoked` = ? and `expires_at` > ?) as `active_tokens_count` from `users` where (LOWER(`users`.`id`) LIKE ? or LOWER(`users`.`email`) LIKE ? or LOWER(`users`.`coverid`) LIKE ? or LOWER(`users`.`zip`) LIKE ? or LOWER(`users`.`origin`) LIKE ? or LOWER(`users`.`id`) LIKE ?) and `users`.`deleted_at` is null

array(8) {
  [0]=>
  bool(false)
  [1]=>
  object(Carbon\Carbon)#455 (3) {
    ["date"]=>
    string(26) "2018-01-31 21:52:20.436981"
    ["timezone_type"]=>
    int(3)
    ["timezone"]=>
    string(13) "Europe/Berlin"
  }
  [2]=>
  string(3) "%t%"
  [3]=>
  string(3) "%t%"
  [4]=>
  string(3) "%t%"
  [5]=>
  string(3) "%t%"
  [6]=>
  string(3) "%t%"
  [7]=>
  string(3) "%t%"
}

This is OK and works. I assume this is the data query.

However, it then goes through prepareCountQuery (to fetch the total filtered count), which overwrites the select part:

if (! $this->isComplexQuery($builder)) {
$row_count = $this->wrap('row_count');
$builder->select($this->connection->raw("'1' as {$row_count}"));
}

This results in the following query:

select '1' as `row_count` from `users` 
where (LOWER(`users`.`id`) LIKE ? or LOWER(`users`.`email`) LIKE ? or 
LOWER(`users`.`coverid`) LIKE ? or LOWER(`users`.`zip`) LIKE ? or 
LOWER(`users`.`origin`) LIKE ? or LOWER(`users`.`id`) LIKE ?) and 
`users`.`deleted_at` is null

Would also look good, but the bindings are the same as above. However, the subselect with 2 params was overwritten, so there are too many bindings (leftovers) and they get matched to the wrong params.

System details

  • Operating System: Homestead Ubuntu
  • PHP Version: 7.1
  • Laravel Version: 5.4
  • Laravel-Datatables Version: 8.1.1
@yajra
Copy link
Owner

yajra commented May 12, 2018

@kronthto @fschalkwijk I think maybe we can include withCount as a complex query hence the subquery will not be overridden with simple select 1..?

Can you please try to update this function:

    /**
     * Check if builder query uses complex sql.
     *
     * @param \Illuminate\Database\Query\Builder $builder
     * @return bool
     */
    protected function isComplexQuery($builder)
    {
        return Str::contains(Str::lower($builder->toSql()), ['union', 'having', 'distinct', 'order by', 'group by', 'join']);
    }

UPDATE
Implemented on PR #1737

@yajra
Copy link
Owner

yajra commented May 12, 2018

On the other hand, it seems working fine with me. Maybe try adding a select statement before the withCount?

User::query()->select()->withCount('activeTokens')

@yajra
Copy link
Owner

yajra commented May 12, 2018

I was able to replicate the issue and @fschalkwijk PR fixes this. Also to further avoid the same issue, I added a PR #1737. Should be fixed on the next release later.

yajra added a commit that referenced this issue May 12, 2018
* 8.0:
  Bump v8.5.1 🚀
  [8.0] Reset select bindings for count query (#1730)
  Classify join statements as a complex query. Fix #1600, #1730, #1471
@github-actions github-actions bot locked and limited conversation to collaborators Oct 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants