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

Obtaining employees and their roles, and vice versa

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

Problem

I am using the following query to obtain a employee with his information, and get all his related roles. The query underneath works fine when we check only 1 location taking only about 0.002 seconds to process, but once we go over ~800 locations it sometimes takes up to 8 full seconds to process the request.

In the actual query I also obtain the roles with all employees bound to them (inverted version of the one below) as a sub query.

Would there be a way to improve this query on higher amounts of locations?
Keys and indexes are already set properly and are also being used correctly.
which originally gave a speed win of 10 seconds (without them it took 18 seconds).

```
SELECT
ifNULL((
GROUP_CONCAT(
'',
'',
'',
-- Ommited variable assignment
ifnull((
SELECT
CONCAT(
'',
GROUP_CONCAT(
'',
'',emp.id,'',
'',
ifnull((
SELECT
CONCAT(
'',
GROUP_CONCAT(
'',
'',
'',
'',
'',
'',
'',
''
SEPARATOR ''
)
,''
)
FROM
photos
INNER JOIN employee_photos
ON photos.id = employee_photos.photo_id and photos.state = 1
WHERE
employee_photos.employee

Solution

Nevermind optimizing the speed of the thing, the most urgent fix is to be able to just read it!

Start with the easy stuff, the FROM clause:

FROM locations AS loc
-- Inner joins not directly relevant to question
INNER JOIN cities ON (loc.city_id=cities.id)
INNER JOIN provinces ON (loc.province_id=provinces.id)
INNER JOIN countries ON (loc.country_id=countries.id)
INNER JOIN regions ON (loc.region_id=regions.id)


Now you can see how many tables are involved, at a glance, without having to mentally sort out INNER JOIN's and ON chunks.

Be consistent with keyword casing. the last ON is spelled on, and the AND in your WHERE clause is spelled and - but when you glance at the script you think all keywords are in YELLCASE. Consistency helps readability there. Same goes for ifnull and ifNULL and is null, which should be IFNULL and ISNULL.

WHERE loc.state = 1 AND loc.id = @locationid


Now, that SELECT statement needs to be broken down. I'd suggest you take each subquery and turn them into distinct views that you can select from, to remove the clutter.

It seems your query is doing two very distinct things: one, it's selecting the data that you need to "serialize" into an XML format. Two, it's "serializing" data into an XML format... manually... by concatenating hard-coded strings.

I'd change the query to do only one of these things, and if possible, use another tool for turning it into XML.

Code Snippets

FROM locations AS loc
-- Inner joins not directly relevant to question
INNER JOIN cities ON (loc.city_id=cities.id)
INNER JOIN provinces ON (loc.province_id=provinces.id)
INNER JOIN countries ON (loc.country_id=countries.id)
INNER JOIN regions ON (loc.region_id=regions.id)
WHERE loc.state = 1 AND loc.id = @locationid

Context

StackExchange Code Review Q#106333, answer score: 5

Revisions (0)

No revisions yet.