views:

93

answers:

4

I have three tables 'Employees', 'Departments' and 'EmployeesInDepartments' The 'Employees' tables references the 'Departments' table (Each employee must have a DepartmentId). However, an employee can exist in multiple departments by adding entries (EmployeeId and DepartmentId) to 'EmployeeInDepartments' table.

I currently have the following stored procedure to retrieve employees by departmentId:

CREATE PROCEDURE dbo.CollectEmployeesByDepartmentId
    (
    @DepartmentId int,
    @IsDeleted bit
    )
AS
BEGIN
        SELECT   Employees.*
        FROM      Employees 
        WHERE   ((Employees.IsDeleted = @IsDeleted )
            AND ((Employees.DepartmentId = @DepartmentId)
                OR (Employees.EmployeeId IN (SELECT EmployeesInDepartments.EmployeeId
                                        FROM EmployeesInDepartments 
                                        WHERE (EmployeesInDepartments.DepartmentId = @DepartmentId)
                                        )
                    )
                )
        )   
END

How can I optimize this stored procedure and possibly use JOINS?

+4  A: 

My first recommendation to you is to remove department Id from the employee table. Insert all records to the employees in Departments table.

Then it's a simple inner join.

And of course, never use select * in production code.

HLGEM
Thanks for the select * recommendation. The change in design has already been proposed but not yet approved. A bit of work may be required to convert current data.
deverop
+4  A: 

Here's my re-write of your query:

WITH summary AS (
   SELECT e.*
     FROM EMPLOYEES e
    WHERE e.isdeleted = @IsDeleted 
      AND e.parentid = 0)
SELECT a.*
  FROM summary a
 WHERE a.departmentid = @DepartmentId
UNION
SELECT b.*
  FROM summary b
  JOIN EMPLOYEESINDEPARTMENTS ed ON ed.employeeid = b.employeeid
                                AND ed.departmentid = @DepartmentId

The UNION is necessary to remove duplicates - if you know there'll never be duplicates, change UNION to UNION ALL.

The CTE called "summary" doesn't provide any performance benefit, it's just shorthand.

OMG Ponies
Thanks for the UNION recommendation.
deverop
+1  A: 
   SELECT E.*
   FROM Employees E
      Left Join EmployeesInDepartments EID ON E.EmployeeId = EID.EmployeeId
         And E.DepartmentId <> @DepartmentId 

   WHERE E.IsDeleted = @IsDeleted
      And
      (  
         E.DepartmentId = @DepartmentId 
         Or (EID.DepartmentId = @DepartmentId)
      )

Edit to include IsDeleted logic. I do agree with some of the other answers, your design should probably be changed. But this query should do it. If you have duplicates in EmployeesInDepartments, you can change that to a Select Distinct.

Shlomo
Out of curiosity, is there a perfomance benefit to the JOIN?
deverop
Although this does not have many votes, this is the answer I was looking for.
deverop
A: 

I will suggest you to remove the IN clause used in your query to WHERE EXISTS If you are using IN in your query it means you are not taking benifits of the indexes defined on the table, and the query will perform a complte table scan which impacts the query performance. You can check this thread to convert a IN to WHERE EXISTS

http://stackoverflow.com/questions/1277632/changing-in-to-exists-in-sql

Prakash Kalakoti
-1 They are treated exactly the same in SQL Server http://sqlinthewild.co.za/index.php/2009/08/17/exists-vs-in/
Martin Smith