-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(db-problems): problems solved, db-backup-tar created #41
base: main
Are you sure you want to change the base?
Conversation
|
||
-- 2 | ||
|
||
SELECT c.name, |
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.
Looks good!
INNER JOIN states s ON c.id = s.country_id | ||
GROUP BY c.name; | ||
--2 | ||
SELECT COUNT(*) employees_without_bosses |
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.
✅
src/answers.sql
Outdated
-- 4 | ||
|
||
SELECT supervisor_id, | ||
COUNT(supervisor_id) as count |
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.
Nothing wrong with this one, works as expected. However, I'd have count by employee.id, that would have make the intention a little bit more clear. Anyways, this is totally fine
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.
Sure, it helps interpretate the query by only reading first SELECT statement!
Done!
src/answers.sql
Outdated
-- 5 | ||
|
||
SELECT COUNT(*) list_of_office |
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.
Naming could be improved. I propose offices_in_colorado
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.
Changed!
-- 7 | ||
|
||
WITH counts AS ( |
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 query can be significantly simplified. You already have thE CTE, you could have use that table to easily get the one with most and the one with less employees. There is not need of using having.
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 resolved and refractored it, and it makes sense to avoid having statement.
WITH counts AS (
SELECT o.address, COUNT(o.id) AS count
FROM offices o
INNER JOIN employees e ON e.office_id = o.id
GROUP BY o.address
ORDER BY count
)
(
SELECT * FROM counts
ORDER BY count DESC
LIMIT 1
)
UNION ALL
(
SELECT * FROM counts
LIMIT 1
);
This solution avoids using having, and only makes 1 inner join. Leading to using less data on the queries after the CTE.
) | ||
LIMIT 1 | ||
) | ||
UNION ALL |
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.
Why using UNION ALL
instead of just UNION
?
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.
For better data normalization.
UNION would gives me only one row in case max and min are the same. However client most like would expect to have the max in the first row and the min on the second row.
It's not that much data where it really matters avoiding duplicates, so having both rows makes sense to me to keep consistency in returning 2 rows each request
src/answers.sql
Outdated
e.first_name || ' ' || e.last_name as full_name, | ||
e.email, | ||
e.job_title, | ||
o.name company, |
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.
Try to keep consistency when naming alias. Either use AS
always, or don't ever use it
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.
Done!
SELECT e.uuid,
e.first_name || ' ' || e.last_name as full_name,
e.email,
e.job_title,
o.name as company,
c.name country,
s.name as state,
es.first_name as boss_name
FROM employees e
INNER JOIN offices o ON e.office_id = o.id
INNER JOIN countries c ON c.id = o.country_id
INNER JOIN employees es ON e.supervisor_id = es.id
INNER JOIN states s ON s.id = o.state_id
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.
You missed country, but it's ok.
Thanks for addressing the comments. Looks better now |
No description provided.