HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlMinor

Listing teams by total distance and bonus

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
totalbonusteamsdistancelistingand

Problem

I had to write a SQL query recently to do a few joins and sums, and at the end of it I realized that I have nearly written a storybook. I know the tools to optimize it, but my trouble is the length of it. I am very sure, it can be shortened up, but my little brain refuses to strengthen this belief.

Basically, I want a list of teams ordered by the total Distance + Bonus. The Bonus consists of the team bonus and the bonus of team members (Member Bonus). In order to calculate distance covered by the team, I have to sum up the distance covered by the team members.

The relationship between the tables:

```
SELECT TeamID, Name, Country, Region, OfficeLocation AS [Office Location], Minutes As Steps, Distance, SUM(MemberBonus+TeamBonus) AS Bonus,TeamSize,
ROW_NUMBER() OVER (ORDER BY SUM( ISNULL(Distance,0.0000000) +ISNULL(MemberBonus,0.0000000)+ISNULL(TeamBonus,0.0000000)) DESC) AS Place
FROM
(
-- Sum the Distance, Member Bonus and Team Bonus and assign them a rank based on the sum value.
SELECT TeamID, Name, Country, Region, Minutes, Distance,
ISNULL(MemberBonus,0.0000000) AS MemberBonus, SUM(ISNULL(teamBonusData.BonusPoints,0.0000000)) AS TeamBonus,
OfficeLocation, TeamSize
FROM
(
SELECT
Team.TeamID, Team.Name, Result.Country AS Country, Result.Region,
Result.Minutes AS Minutes,
SUM(ISNULL(Result.MemberBonus,0)) AS MemberBonus,
Result.Distance AS Distance,
TeamSize
FROM
(
SELECT Group1.TeamID, Group1.Name, Country, Region, Minutes, Distance, MemberBonus, TeamSize
FROM
(
--Get a sum of distance covered by the team's members. Only get the data for the active teams ( Status = 1)
SELECT
Team.TeamID, Team.Name, Country.Name AS Country, Region.Name AS Region,
ISNULL(SUM(Activity.Minutes),0) AS Minutes,

Solution

You really should be more consistent with your choice of line breaks and indentation, especially in a query as large as this. Improving the readability should also make it easier to spot areas for improvement. You didn't indicate which DBMS you're using, but if it's supported, the WITH clause ("common table expressions" or "subquery factoring clause") can help reduce the number of indentation levels you have to deal with.

I feel like you're using ISNULL too much. Remember that aggregate functions like SUM ignore NULL values, so an expression like SUM(ISNULL(Result.MemberBonus,0)) is equivalent to SUM(Result.MemberBonus).

A big problem I see is that you join some tables and subqueries too early. For example, Country and Region appear to be necessary only for information and don't actually affect any calculations. Furthermore, Country and Region only depend on the TeamID. Since that's the case, instead of joining them in the inner-most subquery, join them as part of the outer query. This will shorten up the GROUP BY clauses.

The steps I would take to write this query:

  • Write a query which calculates the distance by team. The only output columns should be TeamID and TotalDistance.



  • Write a query which calculates the total Member bonus by team. The only output columns should be TeamID and TotalMemberBonus.



  • Write a query which calculates the total team bonus by team. The only output columns should be TeamID and TotalTeamBonus.



  • Join the first three queries on TeamID, along with any other information tables like Country and Region. Note that all the aggregation was done in the subqueries, so this final query should not need a GROUP BY clause.



If you follow this outline, you shouldn't need to calculate a ROW_NUMBER to sort the results. Just ORDER BY TotalDistance + TotalMemberBonus + TotalTeamBonus.

The advantage of this approach is that you can test each individual query separately to ensure that you are getting correct results at each step. It also should result in a shorter, easier-to-follow query.

Context

StackExchange Code Review Q#12048, answer score: 8

Revisions (0)

No revisions yet.