-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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.3 Query Builder count() Sql error - PR proposal #15232
Comments
That's not how the |
This |
@themsaid Thanks for pointing that out. I don't know how that got in there either, but if I install 5.3 from composer from scratch I still get the issue
The two fixes either seem to be:
public function aggregate($function, $columns = ['*'])
{
$this->aggregate = compact('function', 'columns');
$this->backupFieldsForCount();
$results = $this->get($columns);
// Once we have executed the query, we will reset the aggregate property so
// that more select queries can be executed against the database without
// the aggregate value getting in the way when the grammar builds it.
$this->aggregate = null;
$this->restoreFieldsForCount();
if (! $results->isEmpty()) {
return array_change_key_case((array) $results[0])['aggregate'];
}
} Number 1 will work, but I wonder if number 2 would be suitable without breaking the other aggregates? |
This query
shouldn't have any problems in strict mode, I tested on MySQL though, not sure about MariaDB. Can you please test using MySQL? |
Curiously, if I run the query above manually (Sequel Pro) on MariaDB it works fine, it's just when it's being executed through Laravel it is failing. Failing on MySQL 5.5.5-10.1.14-MariaDB (Valet) Works fine through Laravel on MySQL 5.7.12 (Homestead) |
That's really confusing, it should never fail, can't think of a sql mode where this would fail :) |
The query generated by laravel is valid (MySQL 5.5.5 & 5.7.12) in strict mode, I believe there's some sort of misconfiguration in your database setup. I think you should just disable strict mode or check your engine configuration. |
FYI - I hit this issue too with Laravel 5.3.26 using 10.1.14-MariaDB on PHP 7.0.7.
If I run that exact query in a
Perhaps you are not supposed to order by when counting in strict mode? I guess it makes sense not to sort as it has no bearing on the count. My use case is that I am building a query in one part of the application and then a separate system uses the query builder instance twice - performing a count for total and then performing a query returning limited results. |
'strict' => false, worked for me. |
|
'strict' => false, worked for me as well. |
Same issue, same solution. |
Sorry for commenting on such an old issue, but I think I can add some more information. It might be useful as it still applies today. The problem is caused by the sql mode The surprising thing is that it only happens with MariaDB, not MySQL. I'm currently running MariaDB 10.2.24 and get the above error, when running this query: SET sql_mode = 'ONLY_FULL_GROUP_BY';
select count(*) as aggregate from `forum_messages` where `forum_messages`.`topic_id` = 20 and `forum_messages`.`topic_id` is not null order by `created_at` asc The same query on MySQL 8.0.2 runs just fine! |
This might be related to #14997.
After upgrading to Laravel 5.3, I get the following Sql error.
I think i've traced it down to the fact that in my repository, I call count() on an already created Builder instance which has an order by (to create a manual paginator for DataTables).
The Query\Builder class has the following method:
I propose changing to the following, which works, using the existing paginator helper methods.
Does anyone know if that will cause any problems with the
count()
,min()
,max()
oraverage()
methods / queries?David.
The text was updated successfully, but these errors were encountered: