SQL Server This stored procedure taking very long, how to improve

Hey I have the following Stored Procedure, which works in steps

1. Fetch all Users which doesnt take to much time
2. Then loop through all users and check how many steps they completed in a test

Its the second step that seems to take ages, because for each user I perform a select query on a pretty large table.

In terms of size There is around 4000 users and 90000 rows in my tblUserQuestionnaireHistory.

Heres the SP

    ALTER PROCEDURE [spGetStoreTrainingSummary_Test]
(
    @staffId INT = default,
    @storeTypeId INT = default,
    @storeId INT = default,
    @county VARCHAR(50) = default,
    @programmeId INT = default,
    @profileId INT = default,
    @showNulls INT = default,
    @position VARCHAR(50) = default,
    @roaId INT = default
)
AS
BEGIN

SET NOCOUNT ON;

-- Place all users inner join stores into a temp table
CREATE TABLE #TempMainTable(
    [id] INT,
    [profileId] INT,
    [position] VARCHAR(50),
    [storeId] INT,
    [county] VARCHAR(50),
    [storeTypeId] INT,
    [roaId] INT
)

INSERT  #TempMainTable
SELECT  tblUsers.id, tblUsers.profileId, tblUsers.position, tblStores.id as storeId, tblStores.county, tblStores.storeTypeId, tblStores.roaID
FROM    tblUsers INNER JOIN tblStores ON tblUsers.storeId = tblStores.id
WHERE   (tblUsers.statusId = 1) AND (tblStores.statusId = 1)

IF @profileId > 0 --## Filter by profile
    BEGIN
        DELETE FROM #TempMainTable WHERE profileId <> @profileId
    END

IF len(@position) > 0 --## Filter by position
    BEGIN
        DELETE FROM #TempMainTable WHERE position <> @position
    END

IF @storeId > 0 --## Filter by store
    BEGIN
        DELETE FROM #TempMainTable WHERE storeId <> @storeId
    END

IF len(@county) > 0 --## Filter by county
    BEGIN
        DELETE FROM #TempMainTable WHERE county <> @county
    END

IF @storeTypeId > 0 --## Filter by storeTypeId
    BEGIN
        DELETE FROM #TempMainTable WHERE storeTypeId <> @storeTypeId
    END

IF @roaId > 0 --## Filter by roaId
    BEGIN
        DELETE FROM #TempMainTable WHERE roaId <> @roaId
    END
-- SELECT * FROM #TempMainTable

CREATE TABLE #TempTable(
    [userId] INT,
    [menuName] varchar(250),
    [stepId] int,
    [programmeId] int,
    [result] varchar(250)
)

DECLARE @UserId INT
DECLARE @MaxStep INT
DECLARE @Result VARCHAR(100)
DECLARE @UserList CURSOR

SET @UserList = CURSOR FOR
SELECT  id
FROM    #TempMainTable
GROUP BY id

OPEN @UserList

FETCH NEXT FROM @UserList INTO @UserId
WHILE (@@FETCH_STATUS = 0)  
BEGIN
        --## Staff Induction Programme
        SET @MaxStep = (SELECT  MAX(stepId) AS maxId FROM tblUserQuestionnaireHistory WHERE (userId = @UserId) AND (programmeId = 13) AND (success = 1))
        SET @Result = (SELECT CASE WHEN @MaxStep = 9 THEN 'Passed' WHEN @MaxStep <> 9 THEN 'Step ' + + CAST(@MaxStep AS VARCHAR(10)) + ' completed out of 9' ELSE 'No steps completed yet' END as Result)
        INSERT #TempTable
        SELECT @UserId, 'Staff Induction Programme &copy;', @MaxStep, 13, @Result
    --PRINT @UserId
        --PRINT @MaxStep
        --PRINT @Result

FETCH NEXT FROM @UserList INTO @UserId
END 
CLOSE @UserList
DEALLOCATE @UserList
DROP TABLE #TempMainTable

--## Filter by programme id
IF @programmeId IS NOT NULL
BEGIN
    DELETE FROM #TempTable WHERE programmeId <> @programmeId
END

IF @showNulls = 1 -- Select All Records
BEGIN
    SELECT #TempTable.*, tblUsers.firstName + ' ' + tblUsers.lastName AS fullname, tblUsers.firstName, tblUsers.lastName, tblStores.name AS storeName, tblStores.county, tblStore_Types.name AS storeType, tblUsers.position, tblStores.surname + ' ' + tblStores.name as retailerName, tblProfiles.name as profile, tblRoa.lastname + ' ' + tblRoa.firstname as roa
    FROM #TempTable INNER JOIN tblUsers ON #TempTable.userId = tblUsers.id INNER JOIN tblStores ON tblUsers.storeId = tblStores.id INNER JOIN tblStore_Types ON tblStores.storeTypeId = tblStore_Types.id INNER JOIN tblProfiles ON tblUsers.profileId = tblProfiles.id INNER JOIN tblROA ON tblStores.roaID = tblROA.id
END
ELSE IF @showNulls = 2 -- Select Users who have sat at least one training
BEGIN
    SELECT #TempTable.*, tblUsers.firstName + ' ' + tblUsers.lastName AS fullname, tblUsers.firstName, tblUsers.lastName, tblStores.name AS storeName, tblStores.county, tblStore_Types.name AS storeType, tblUsers.position, tblStores.surname + ' ' + tblStores.name as retailerName, tblProfiles.name as profile, tblRoa.lastname + ' ' + tblRoa.firstname as roa 
    FROM #TempTable INNER JOIN tblUsers ON #TempTable.userId = tblUsers.id INNER JOIN tblStores ON tblUsers.storeId = tblStores.id INNER JOIN tblStore_Types ON tblStores.storeTypeId = tblStore_Types.id INNER JOIN tblProfiles ON tblUsers.profileId = tblProfiles.id INNER JOIN tblROA ON tblStores.roaID = tblROA.id
    WHERE userId IN (SELECT userId FROM #TempTable WHERE (stepId IS NOT NULL) GROUP BY userId)
END
ELSE -- Select Only Training records that have been sat
BEGIN 
    SELECT #TempTable.*, tblUsers.firstName + ' ' + tblUsers.lastName AS fullname, tblUsers.firstName, tblUsers.lastName, tblStores.name AS storeName, tblStores.county, tblStore_Types.name AS storeType, tblUsers.position, tblStores.surname + ' ' + tblStores.name as retailerName, tblProfiles.name as profile, tblRoa.lastname + ' ' + tblRoa.firstname as roa
    FROM #TempTable INNER JOIN tblUsers ON #TempTable.userId = tblUsers.id INNER JOIN tblStores ON tblUsers.storeId = tblStores.id INNER JOIN tblStore_Types ON tblStores.storeTypeId = tblStore_Types.id INNER JOIN tblProfiles ON tblUsers.profileId = tblProfiles.id INNER JOIN tblROA ON tblStores.roaID = tblROA.id 
    WHERE (stepId IS NOT NULL)
END

END

Any hints on how I can approve this stored procedure ?

EDIT TO SHOW LATEST SP :

    ALTER PROCEDURE [u1017987_dbase_user].[spGetStoreTrainingSummary_Test]
(
    @staffId INT = default,
    @storeTypeId INT = default,
    @storeId INT = default,
    @county VARCHAR(50) = default,
    @programmeId INT = default,
    @profileId INT = default,
    @showNulls INT = default,
    @position VARCHAR(50) = default,
    @roaId INT = default
)
AS
BEGIN

SET NOCOUNT ON;

-- Place all users inner join stores into a temp table
CREATE TABLE #TempMainTable(
    [id] INT,
    [profileId] INT,
    [position] VARCHAR(50),
    [storeId] INT,
    [county] VARCHAR(50),
    [storeTypeId] INT,
    [roaId] INT
)



CREATE TABLE #TempTable(
    [userId] INT,
    [menuName] varchar(250),
    [stepId] int,
    [programmeId] int,
    [result] varchar(250)
)

DECLARE @UserId INT
DECLARE @MaxStep INT
DECLARE @Result VARCHAR(100)
DECLARE @UserList CURSOR

;WITH tempMainTable
AS
(
  SELECT  tblUsers.id, tblUsers.profileId, tblUsers.position, tblStores.id as storeId, 
  tblStores.county, tblStores.storeTypeId, tblStores.roaID
  FROM    tblUsers INNER JOIN tblStores ON tblUsers.storeId = tblStores.id
  WHERE   (tblUsers.statusId = 1) AND (tblStores.statusId = 1)
  AND (@profileId = 0 OR profileId = @profileId)
  AND (len(@position) = 0 OR position = @position)
  AND (@storeId = 0 OR storeId = @storeId)
  AND (len(@county) = 0 OR county = @county)
  AND (@storeTypeId = 0 OR storeTypeId = @storeTypeId)
  AND (@roaId = 0 OR roaId = @roaId)
),
tempTable AS
(
    SELECT tempMainTable.userId,
           'Staff Induction Programme ©',
           (SELECT  MAX(stepId) AS maxId FROM tblUserQuestionnaireHistory WHERE (userId = tempMainTable.userId) AND (programmeId = 13) AND (success = 1)),
           13,
           (SELECT CASE (SELECT  MAX(stepId) AS maxId FROM tblUserQuestionnaireHistory WHERE (userId = tempMainTable.userId) AND (programmeId = 13) AND (success = 1)) WHEN  9 THEN 'Passed' ELSE 'Step ' + + CAST(@MaxStep AS VARCHAR(10)) + ' completed out of 9'  END as Result)    
    FROM tempMainTable
    WHERE (@programmeId IS NULL OR @programmeId=13)
)

--## Filter by programme id
IF @programmeId IS NOT NULL
BEGIN
    DELETE FROM #TempTable WHERE programmeId <> @programmeId
END

IF @showNulls = 1 -- Select All Records
BEGIN
    SELECT #TempTable.*, tblUsers.firstName + ' ' + tblUsers.lastName AS fullname, tblUsers.firstName, tblUsers.lastName, tblStores.name AS storeName, tblStores.county, tblStore_Types.name AS storeType, tblUsers.position, tblStores.surname + ' ' + tblStores.name as retailerName, tblProfiles.name as profile, tblRoa.lastname + ' ' + tblRoa.firstname as roa
    FROM #TempTable INNER JOIN tblUsers ON #TempTable.userId = tblUsers.id INNER JOIN tblStores ON tblUsers.storeId = tblStores.id INNER JOIN tblStore_Types ON tbl开发者_运维百科Stores.storeTypeId = tblStore_Types.id INNER JOIN tblProfiles ON tblUsers.profileId = tblProfiles.id INNER JOIN tblROA ON tblStores.roaID = tblROA.id
END
ELSE IF @showNulls = 2 -- Select Users who have sat at least one training
BEGIN
    SELECT #TempTable.*, tblUsers.firstName + ' ' + tblUsers.lastName AS fullname, tblUsers.firstName, tblUsers.lastName, tblStores.name AS storeName, tblStores.county, tblStore_Types.name AS storeType, tblUsers.position, tblStores.surname + ' ' + tblStores.name as retailerName, tblProfiles.name as profile, tblRoa.lastname + ' ' + tblRoa.firstname as roa 
    FROM #TempTable INNER JOIN tblUsers ON #TempTable.userId = tblUsers.id INNER JOIN tblStores ON tblUsers.storeId = tblStores.id INNER JOIN tblStore_Types ON tblStores.storeTypeId = tblStore_Types.id INNER JOIN tblProfiles ON tblUsers.profileId = tblProfiles.id INNER JOIN tblROA ON tblStores.roaID = tblROA.id
    WHERE userId IN (SELECT userId FROM #TempTable WHERE (stepId IS NOT NULL) GROUP BY userId)
END
ELSE -- Select Only Training records that have been sat
BEGIN 
    SELECT #TempTable.*, tblUsers.firstName + ' ' + tblUsers.lastName AS fullname, tblUsers.firstName, tblUsers.lastName, tblStores.name AS storeName, tblStores.county, tblStore_Types.name AS storeType, tblUsers.position, tblStores.surname + ' ' + tblStores.name as retailerName, tblProfiles.name as profile, tblRoa.lastname + ' ' + tblRoa.firstname as roa
    FROM #TempTable INNER JOIN tblUsers ON #TempTable.userId = tblUsers.id INNER JOIN tblStores ON tblUsers.storeId = tblStores.id INNER JOIN tblStore_Types ON tblStores.storeTypeId = tblStore_Types.id INNER JOIN tblProfiles ON tblUsers.profileId = tblProfiles.id INNER JOIN tblROA ON tblStores.roaID = tblROA.id 
    WHERE (stepId IS NOT NULL)
END

END

---

Ok, I'll try to break this down as much as possible.

1) Temp tables are quite slow, you'd get instantly better performance using CTE's
2) Cursors in SQL are insanely slow, much of this logic should probably go into your Business layer.

The first Temp table and associated DELETE's can be your first CTE, and you dont need all that logic, just a decent set op

;WITH tempMainTable
AS
(
  SELECT  tblUsers.id, tblUsers.profileId, tblUsers.position, tblStores.id as storeId, 
  tblStores.county, tblStores.storeTypeId, tblStores.roaID
  FROM    tblUsers INNER JOIN tblStores ON tblUsers.storeId = tblStores.id
  WHERE   (tblUsers.statusId = 1) AND (tblStores.statusId = 1)
  AND (@profileId = 0 OR profileId = @profileId)
  AND (len(@position) = 0 OR position = @position)
  AND (@storeId = 0 OR storeId = @storeId)
  AND (len(@county) = 0 OR county = @country)
  AND (@storeTypeId = 0 OR storeTypeId = @storeTypeId)
  AND (@roaId = 0 OR roaId = @roaId)
),
tempTable AS
(
    SELECT tempMainTable.userId,
           'Staff Induction Programme ©',
           (SELECT  MAX(stepId) AS maxId FROM tblUserQuestionnaireHistory WHERE (userId = tempMainTable.userId) AND (programmeId = 13) AND (success = 1)),
           13,
           (SELECT CASE (SELECT  MAX(stepId) AS maxId FROM tblUserQuestionnaireHistory WHERE (userId = tempMainTable.userId) AND (programmeId = 13) AND (success = 1)) WHEN  9 THEN 'Passed' WHEN ELSE 'Step ' + + CAST(@MaxStep AS VARCHAR(10)) + ' completed out of 9'  END as Result)    
    FROM tempMainTable
    WHERE (@programmeId IS NULL OR @programmeId=13)
)
// do the rest here

That immediately gets rid of the need for the first temp table, the second and the cursor.

But the main advantage here I think is in not filling lots of data and then going through deleting it bit by bit. Start off with the righ tdata set by filtering the data based on your parameters first as I have done in the first CTE above,

---

You seem to know that the cursor is your issue. You probably need to turn it into set operations. This is sort of my general approach into how to convert cursor ops into set ops

1. create a temp table ready to hold all data
2. repeatedly fill the temp table with the data/computations as needed
3. Take the temp data and perform CRUD on the db as needed.

From the source code my suggestion is to break up the curso loop into

1. Set operation 1 : Get all users into a temp table (that has correct columns for all the data you'll collect)
2. Set operation 2 : Add the @MaxStep and @result to the temp table (for al users)
3. Set operation 3 : if @programmeId IS NOT NULL do the correct set operation
4. Set operation 4 : if next variable = whatever do the correct set operation ... rinse and repeat etc...

From the final temp table, do the correct selects.

---

One likely culprit is the cursor. Try to rewrite it as a set based operation. For example:

INSERT  #TempTable
SELECT  tmt.id
,       'Staff Induction Programme ©'
,       uqh.MaxStep
,       13
,       CASE 
        WHEN uqh.MaxStep = 9 THEN 'Passed' 
        WHEN uqh.MaxStep <> 9 THEN 'Step ' + cast(uqh.MaxStep AS VARCHAR(10)) + 
                                   ' completed out of 9' 
        ELSE 'No steps completed yet' 
        END
FROM    #TempMainTable tmt
LEFT JOIN
        (
        SELECT  uqh.userId
        ,       MAX(uqh.stepId) as MaxStep
        FROM    tblUserQuestionnaireHistory uqh 
        WHERE   uqh.programmeId = 13 
                AND uqh.success = 1
        GROUP BY
                uqh.userId
        ) uqh
on      uqh.userId = tmt.id