5
\$\begingroup\$

The below process is designed to pick out only AsbestosUPRNs and get the lowest OverallRiskCategory where they have the newest SurveyDate. I just wanted to ensure this is the best way, it seems neat now though as well as second code, which in reality is the same thing, based on two tables.

SELECT 
AsbestosUPRN, 
MIN(OverallRiskCategory) AS OverallRiskCategory, 
MAX(SurveyDate) AS SurveyDate
FROM TblAsbestos 
GROUP BY 
AsbestosUPRN

Two Table version:

SELECT
p.UCARN, 
MAX(a.OverallRiskCatNumberical) as OverallRiskCatNumberical, 
MAX(a.SurveyDate) AS SurveyDate
FROM TblAsbestos AS a
INNER JOIN TblProperty AS p
ON p.UPRN = a.AsbestosUPRN 
GROUP BY 
p.UCARN
\$\endgroup\$
6
\$\begingroup\$

Overall, that looks good. You've followed basic SQL styling rules, capitalising keywords for example, and the both queries are consistent. Anyone looking at that is going to struggle to find things to complain about.

A couple little things, the first AS in your second query isn't capitalised. I would change it to AS to make the query more consistent.

I'd say that the table aliases in the second query should be more meaningful than just a and p. Other than that, good job.

I would recommend using the first query, it avoids the join and, presumably, returns the same correct results.

\$\endgroup\$
  • \$\begingroup\$ Thanks, nice spot on the 'AS'. I had to use a join on the second on as the data I needed was on the second table. \$\endgroup\$ – indofraiser Sep 9 '14 at 14:14
4
\$\begingroup\$

Not much to criticize

Assuming both versions return the same results, I would pick the former. However I do recommend a sanity check to ensure that, and select the one that returns the correct result set.

Good things

Your code is KISS and DRY. Nothing seems ambiguous to me. You could make it more explicit by adding more ACID stuff, like setting the transaction isolation level and such, but for something simple like this it seems like overkill.

Your capitalization is consistent (aside from that tiny as pointed by @PenutReaper). I might recommend a slight indentation to break down which statements are part of a single section to make it a bit easier to read, as such:

SELECT 
    AsbestosUPRN, 
    MIN(OverallRiskCategory) AS OverallRiskCategory, 
    MAX(SurveyDate) AS SurveyDate
FROM 
    TblAsbestos 
GROUP BY 
    AsbestosUPRN

The short version:

Nice.

\$\endgroup\$
  • 1
    \$\begingroup\$ The indentation you suggest is a pretty good choice, as long as your column names are not too long. For this query I see no problem with it. I would typically suggest people use whatever they are comfortable with and that they find readable on their particular PC/editor setup. \$\endgroup\$ – PenutReaper Sep 10 '14 at 8:48

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.