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

Database count methods #2159

Closed
MGatner opened this issue Aug 23, 2019 · 6 comments
Closed

Database count methods #2159

MGatner opened this issue Aug 23, 2019 · 6 comments

Comments

@MGatner
Copy link
Member

MGatner commented Aug 23, 2019

We don't have any way to perform count() calls from the query builder. This is a necessary method for running any grouped queries with multiple returns. Take the native migrations table:

+----+-------------------+-------------------------------------------------------------------------+---------+-------------------+------------+-------+
| id | version           | class                                                                   | group   | namespace         | time       | batch |
+----+-------------------+-------------------------------------------------------------------------+---------+-------------------+------------+-------+
|  1 | 2018-01-24-102301 | App\Database\Migrations\Migration_some_migration                        | default | App               | 1566095631 |     1 |
|  2 | 2018-01-24-102302 | App\Database\Migrations\Migration_another_migration                     | default | App               | 1566095631 |     1 |
| 30 | 20190327133719    | App\Database\Migrations\Migration_create_table_careers                  | default | App               | 1566499960 |     2 |
| 31 | 20171120223112    | Myth\Auth\Database\Migrations\Migration_create_auth_tables              | default | Myth\Auth         | 1566499962 |     3 |
| 34 | 20190326110032    | Tatter\Permits\Database\Migrations\Migration_create_table_permits       | default | Tatter\Permits    | 1566499962 |     3 |
| 38 | 20190425103401    | Tatter\Workflows\Database\Migrations\Migration_create_table_workflows   | default | Tatter\Workflows  | 1566499962 |     3 |
| 39 | 20190425103427    | Tatter\Workflows\Database\Migrations\Migration_create_table_jobs        | default | Tatter\Workflows  | 1566499962 |     3 |
| 40 | 20190425103555    | Tatter\Workflows\Database\Migrations\Migration_create_table_stages      | default | Tatter\Workflows  | 1566499962 |     3 |
| 32 | 2019-08-22-214305 | Tatter\Exports\Database\Migrations\Migration_create_health_tables       | default | Tatter\Health     | 1566500702 |     4 |
| 33 | 20190319121802    | Tatter\Visits\Database\Migrations\Migration_create_table_visits         | default | Tatter\Visits     | 1566500702 |     4 |
+----+-------------------+-------------------------------------------------------------------------+---------+-------------------+------------+-------+

If I want to know how many migrations were run in each batch, I'd query SELECT batch, COUNT(*) FROM migrations GROUP BY batch. Slightly more complex, if I want to know how many namespaces were part of each batch, I'd query SELECT batch, COUNT(DISTINCT(namespace)) FROM migrations GROUP BY batch.

I run these kind of queries a lot for reporting and summarizing and was surprised to see there is no way to handle this in the query builder. I would propose selectCount() and selectCountDistinct() as additional methods implemented similar to selectMin(), selectMax(), and selectAvg().

I am willing to make an attempt at these but my database experience is heavily weighted in MySQL and I'd want someone with more knowledge of other handlers to collaborate.

@lonnieezell
Copy link
Member

What about countAllResults() ?

@MGatner
Copy link
Member Author

MGatner commented Aug 23, 2019

That simply returns the number of rows returned, not variance within each grouping. I should have provided results from my two examples above.
SELECT batch, COUNT(*) FROM migrations GROUP BY batch

+-------+----------+
| batch | count(*) |
+-------+----------+
|     1 |        2 |
|     2 |        1 |
|     3 |       14 |
|     4 |        1 |
+-------+----------+

SELECT batch, COUNT(DISTINCT(namespace)) FROM migrations GROUP BY batch

+-------+----------------------------+
| batch | count(distinct(namespace)) |
+-------+----------------------------+
|     1 |                          1 |
|     2 |                          1 |
|     3 |                         11 |
|     4 |                          1 |
+-------+----------------------------+

In both these scenarios countAllResults() will just return 4.

@lonnieezell
Copy link
Member

Oh! I see what you're saying. And you don't want to build that query manually and just use query()? :)

I'm cool with a count() method being added to the Query Builder. I always dread adding stuff to the database layer because of the complexities, so be fore-warned. And my experience is almost exclusively in MySQL, also, but I've got the others installed locally and can read docs with the best of them lol

@MGatner
Copy link
Member Author

MGatner commented Aug 23, 2019

Well I'm not opposed to using query() but it seems like a "gap" given that the Query Builder can already groupBy() and handle the other aggregation functions (which are arguably more complex).

I'll take a look at how the handlers approach selectSum() etc and see if this is going to be a can of worms (4.1) or an easy PR. Thanks for entertaining :)

@MGatner
Copy link
Member Author

MGatner commented Aug 23, 2019

BaseBuilder.php:194 makes me think that COUNT() is available across all handlers: protected $countString = 'SELECT COUNT(*) AS ';

I think I will add a COUNT option to maxMinAvgSum() for now (unless you're opposed) and leave COUNT(DISTINCT()) for another time since it seems like that is handler-specific.

@lonnieezell
Copy link
Member

I think that's fine for now.

@jim-parry jim-parry added this to the 4.0.0-rc.2 milestone Sep 8, 2019
@MGatner MGatner closed this as completed Sep 14, 2019
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

3 participants