1
\$\begingroup\$

I've inherited an Access database and am looking to start using it to do reporting. I've written a query that gets me the information I'm looking for, I'm just hoping someone can have a look over it and tell me if I'm doing it the wrong way or not.

Anyway, the query takes information from 3 tables:

  • sites - more detailed information on each workplace
  • employees - list of employees and their associated workplace
  • cases - holds the bulk of the information needed for the report

I want most fields from the cases table as this is how work is tracked, but I want to also group by the city which is in the sites table and by employee name from the employees table.

SELECT 
    sites.city, 
    employees.[first name], 
    employees.[last name], 
    cases.[assigned to], 
    cases.ref, 
    cases.status, 
    cases.priority, 
    cases.[opened date], 
    Round(Now()-cases.[opened date],0) AS Age
FROM
    cases 
LEFT JOIN 
    (employees LEFT JOIN sites ON employees.site = sites.site) 
ON 
    cases.[assigned to] = employees.username
WHERE 
    cases.[assigned to]<>"Not Assigned" 
AND 
    cases.status="Active" 
AND 
    cases.priority<>"first contact"
ORDER BY 
    sites.city, 
    employees.[first name], 
    employees.[last name], 
    cases.[opened date] desc;

Is there any stupid rookie mistakes in here that I've fallen prey to? Could that be done any better? Have I not given enough information in my question?

\$\endgroup\$
0
2
\$\begingroup\$

I can't see any major flaw in this query.

I would just make the Jointures like this, as it's more clear :

SELECT 
    sites.city, 
    employees.[first name], 
    employees.[last name], 
    cases.[assigned to], 
    cases.ref, 
    cases.status, 
    cases.priority, 
    cases.[opened date], 
    Round(Now()-cases.[opened date],0) AS Age
FROM ((cases 
LEFT JOIN employees ON cases.[assigned to] = employees.username)
LEFT JOIN sites ON employees.site = sites.site)
WHERE cases.[assigned to]<>"Not Assigned" 
  AND cases.status="Active" 
  AND cases.priority<>"first contact"
ORDER BY 
    sites.city, 
    employees.[first name], 
    employees.[last name], 
    cases.[opened date] desc;

If you can't have any case with non existing employees and sites, it's better to change your 2 LEFT JOIN into INNER JOIN. Just throw the 2 queries and compare the results.

In your WHERE clause, having some <> is generally inefficient on large tables. It's always better to make a IN() and list all the possibilities excluding "Not assigned" or "first contact" :

  1. If cases.[assigned to] can only contain an employee username or "Not Assigned", then just doing an INNER JOIN on the employee table will automatically filter the not assigned and you don't need to specify it in the WHERE clause.
  2. if you have a limited number of case.priority values, list them all but "first contact" : AND cases.priority IN ("priority1", "super urgent", "can wait")

For the Age, if you want to compute the days from your open_date, it's better to do :

dateDiff("d",Now(),cases.[opened date]) AS Age

you can also concatenate the first and last name in one column:

 (employees.[last name] & ' ' & employees.[first name]) AS EmpName,

That's all what comes to my mind.

\$\endgroup\$
2
  • \$\begingroup\$ I appreciate the tips. I had always assumed a NOT clause would be less taxing because it only dealt with one value instead of several. I'll have to do more reading. \$\endgroup\$ Sep 12 '16 at 2:28
  • \$\begingroup\$ The different than operator is a perf killer for all RDBMS I know. It often results in full table scan \$\endgroup\$
    – Thomas G
    Sep 12 '16 at 13:53
1
\$\begingroup\$

The nested joins are unconventional. A "flat chain" would be the more common and readable way to write it.

FROM cases 
    LEFT JOIN employees
        ON cases.[assigned to] = employees.username
    LEFT JOIN sites
        ON employees.site = sites.site

SQL string literal a should be written using single quotes. Double quotes may be accepted by MS Access (and MySQL), but they are nonstandard and could be interpreted differently in standard-compliant databases.

WHERE 
    cases.[assigned to] <> 'Not Assigned' AND 
    cases.status = 'Active' AND 
    cases.priority <> 'first contact'

You shouldn't be using 'Not Assigned' as a special value, though. It's bad practice to use values with special significance that might also be valid data — even if this special value is less likely to occur than 'Null' or 'XXXXXXX'. If there really isn't anyone assigned to the case, use a NULL to indicate that fact, and use WHERE cases.[assigned to] IS NOT NULL in the query.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the feedback. I inherited the design where 'Not Assigned' was a value instead of null which does seem like the logical choice. I'll certainly be looking to change it in the future. \$\endgroup\$ Sep 12 '16 at 2:22

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.