3
\$\begingroup\$

I have this query where I am working out the percentage of peoples occupation. I have a sub query that works out the total count of people codes and that result is what I used to divide by. I could hardcode this but the count could change.

It just looks a bit messy and I am wondering if there is a nicer way to do the sub query.

SELECT
    CONVERT(VARCHAR(20),COUNT(CO.code)*100 / 
(
SELECT 
COUNT(CO.Code)
FROM 
TABLE1 K
INNER JOIN TABLE2 CO ON(CO.id= K.id AND CO.status= 'Something' AND K.statuscode = 0  AND CO.statuscode =0))
)+'%',

Here is the final query. I needed the sub query because of the group by which if I did the count in the SELECT, it would just return the number of people who had a specific occupation.

SELECT
    CONVERT(VARCHAR(20),COUNT(CO.code)*100 / 
(
SELECT 
COUNT(CO.Code)
FROM 
TABLE1 K
INNER JOIN TABLE2 CO ON(CO.id= K.id AND CO.status= 'Something' AND K.statuscode = 0  AND CO.statuscode =0))
)+'%',
K.occupation
FROM
TABLE 1 K
LEFT OUTER TABLE 2 CO  ON(CO.id = K.id AND CO.status = 'Something' AND K.statuscode = 0 AND CO.statuscode = 0)
GROUP BY
K.occupation
ORDER BY 
COUNT(CO.Code) DESC

Is there a nicer/more efficient way to do this?

\$\endgroup\$
1
  • \$\begingroup\$ You missed the word JOIN on your LEFT OUTER JOIN. \$\endgroup\$ – PenutReaper Oct 24 '14 at 11:47
4
\$\begingroup\$

Aliases

The aliases you have used in this query, K and CO, are meaningless. I would recommend using either the full table name, if it is short enough, or using a meaningful alias, such as People, Jobs or Fruit for a tables containing information regarding People, Jobs and Fruit respectively.

I would recommend always including AS when using an alias to prevent confusion.

X = 0, X= 0 or X =0?

Okay first things first, you have examples of X= 0, X = 0 and X =0. You should choose find a style you like and can read well and stick with it. I'd recommend X = 0. I find it easier to understand where each side of the statement ends with the separating spaces.

Joins

Next, we have this bit:

INNER JOIN TABLE2 CO ON(CO.id= K.id AND CO.status= 'Something' AND K.statuscode = 0  AND CO.statuscode =0))

Firstly, you don't need brackets around the ON part of this. Adding the unnecessary brackets can just end up confusing you. If you had used a combination of ANDs and ORs in the ON clause, then you would use brackets to make sure the statement is executed as you intended.

Secondly, you are checking that both statuscodes are 0 in the ON clause. The correct way to do this would be to make sure you only bring back records where the statuscodes are the same, and check one of the statuscodes in the WHERE clause.

Thirdly, CO.status = 'Something' should also be moved into the WHERE clause.

Lastly, is there a reason that the subquery uses INNER JOIN and the main query uses LEFT OUTER JOIN?

A Suggestion

Might I suggest removing the subquery and calculating the total number of jobs and storing the result in a variable? This should speed things up a little and make the main query a little easier on the eyes.

Bringing It All Together

This is what I ended up with:

DECLARE @Total_Jobs INT = 
(
    SELECT COUNT(CO.Code)
    FROM TABLE1 AS K
    INNER JOIN TABLE2 AS CO 
    ON CO.id = K.id AND CO.statuscode = K.statuscode
    WHERE CO.status = 'Something' 
    AND K.statuscode = 0
)

SELECT CONVERT(VARCHAR,COUNT(CO.code)*100/@Total_Jobs)
FROM TABLE1 AS K
LEFT OUTER JOIN TABLE2 CO  
ON CO.id = K.id AND CO.statuscode = K.statuscode
WHERE CO.status = 'Something' 
AND K.statuscode = 0
GROUP BY K.occupation
ORDER BY COUNT(CO.Code) DESC
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.