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

[Bug] QueryBuilder Sum aggregate not working with group statements #9567

Closed
dsturrock opened this issue Jul 8, 2015 · 8 comments
Closed

Comments

@dsturrock
Copy link

Just had a situation where I was performing SUM on a statement grouped by week and getting back 52 results from MySQL as expected. When running the same thing through a QueryBuilder->sum() function it only returns the first value.

I'd guess it would either want to ignore the group by and SUM the whole lot but it's only returning the SUM for the first week, so it's still using the group by but not returning all results.

@ArjanSchouten
Copy link
Contributor

Can you give some code so we can reproduce things? Have you posted this on a forum?

@dsturrock
Copy link
Author

Sure, so when I run sum() on a query builder instance with code like this:

$result = $builder->sum($columnName);

I'll get a single value back, say 3000.

If I grab the query using toSql() and replicate the inputs in command line MySQL, I get a list of values back:

3000
2875
3530
...

So it's performing the sum correctly but seems to assume it's only going to get one SUM total back, not accounting for the group by applied to the query builder.

@ArjanSchouten
Copy link
Contributor

It is still difficult to reproduce! Have you read this: http://laravel.com/docs/5.1/contributions#bug-reports. Which versions of all software/projects are you using? What is the $builder? What sql does the toSql() ouput?

@dsturrock
Copy link
Author

Currently pointed against laravel/framework 5.1.6

My code won't be too much help here since it's obviously specific to my data-set.

The only steps you should need to be able to reproduce are:

  1. Create an /Illuminate/Database/Query/Builder object using DB::table() or Eloquent->getQuery()
  2. Add a Select, Where and Group By clause to the Builder
  3. perform ->sum($columName) on the builder object and observe the single result returned.

One notable point is the select, where, and group by are defined in separate classes and methods before the Builder is passed to the code that performs the sum().

Output from toSql()

"select * from weekly_tablewhere weekly_table.timestamp between ? and ? group byweek"

so with example timestamps :

"select * from weekly_tablewhere weekly_table.timestamp between 1420070400 and 1436439462 group byweek"

@ArjanSchouten
Copy link
Contributor

Ok digged into this. If you read this pull request #8113 you'll see that there was already a discussion about that. You should do the groupby and after that you've to take the sum.

@dsturrock
Copy link
Author

So from that discussion the issue seems to be the Query Builder aggregate functions pluck single values by default. This does seem like a bug to me since my current workaround is just to replace the ->sum with ->lists(DB::raw("SUM({$columnName})")). With this in mind the Query Builder aggregate function clearly has different behavior to the raw one.

I might suggest having something to make this configurable so they can still default to plucking single values but it should be possible to return the full result for use cases like this.

@jarektkaczyk
Copy link
Contributor

I think it was already discussed long time ago, but I agree that aggregates should adapt automatically to the query with groupBy. It's very unlikely, that you would like single value returned while using group by.

@GrahamCampbell
Copy link
Member

If someone can provide a small test case, please create a new issue.

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