-
Notifications
You must be signed in to change notification settings - Fork 50
Implement Count Queries #515
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
base: feat-sum-queries
Are you sure you want to change the base?
Conversation
// Test mixing count with other queries | ||
$documents = $database->find('testCountQueries', [ | ||
Query::count('integer'), | ||
Query::limit(1), |
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.
Perhaps should validate not using limits / offset queries Since they happen after the aggregations.
Most likely count/sum will return 1 row.
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.
We run aggregate SQL after the main SQL using a subquery so this shouldn't be a problem
$this->assertEquals(1, $documents[0]->getAttribute('integer')); | ||
|
||
$documents = $database->find('testCountQueries', [ | ||
Query::count('integer'), |
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.
- Do we allow select queries when using aggregation queries, which will lead to errors?
- Do we allow Query::count('integer'), Query::sum('integer') will both have the alias?
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.
- Do we validate aggregation queries in nested queries?
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.
- No we do not allow select queries when using aggregation queries, we have a validator for that.
- We don't allow mixing aggregate queries
- I haven't touched nested queries, so I don't think so
This PR implements count queries and generally cleans up the code of how aggregates work so it's more maintainable. This supports all DB adapters excluding MongoDB.