-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
[11.x] Support DB aggregate by group (new methods) #53679
Conversation
$sql = 'select '; | ||
|
||
if ($query->groups) { | ||
$sql .= $this->columnize($query->groups).', '; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds the group columns in the results, even if they are not used.
If a group column is named aggregate
, the DB engine will prefix it with the table name. We don't have conflict with the calculated aggregate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have fear this is going to be another breaking change 😅 any way we can be sure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't affect queries created by the query builder. The result of the aggregate functions count
, max
... will stay the same (#53581).
It only add fields to the result when calling the grammar method directly. I moved the new columns last to keep the aggregate
result first.
4bb788d
to
bc5ca4b
Compare
bc5ca4b
to
bd77811
Compare
This reverts commit 4b9aeae.
Replaces #53209 that have been reverted by #53582.
This new version adds new methods:
countByGroup
,minByGroup
,maxByGroup
,sumByGroup
andavgByGroup
. I did not create the aliasaverageByGroup
, but I can if you believe that's useful.