The below code is run on a routine rather than say login time but being new to using FETCH NEXT I can't help think maybe there is a trick here to merge at least one of the SET lines.
The end results should give me the total unique users, how many completed a module and how many passed.
Fields are indexed on the table at this time but all additional thoughts welcome.
USE [DB]
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[spModuleStatisticsUnique2]
-- Add the parameters for the stored procedure here
AS
DECLARE @i INT = 0;
DECLARE @i INT = 0;
DECLARE @Started int;
DECLARE @Completed int;
DECLARE @Passed int;
DECLARE @ModuleID int;
DECLARE merge_cursor CURSOR FAST_FORWARD FOR SELECT [ModuleID] FROM dbo.[TblModules]
--ORDER BY [ModuleID]
OPEN merge_cursor
FETCH NEXT FROM merge_cursor INTO @ModuleID
WHILE @@FETCH_STATUS = 0
BEGIN
SET @Started = (Select Count(ID) FROM [TblResults] WHERE [ModuleID] = @ModuleID)
SET @Completed = (Select Count(ID) FROM [TblResults] WHERE [ModuleID] = @ModuleID and ModuleDatecomplete <> '')
SET @Passed = (Select Count(ID) FROM [TblResults] WHERE [ModuleID] = @ModuleID and Pass = 'Yes')
UPDATE [TblModules]
SET
[Started] = @Started
,[Completed] = @Completed
,[Passed] = @Passed
,[PassedPercent] = ((@Passed * 100.0) / @Started)
WHERE
[ModuleID] = @ModuleID
FETCH NEXT FROM merge_cursor INTO @ModuleID
END
CLOSE merge_cursor
DEALLOCATE merge_cursor
1 Answer 1
I don't think you need a cursor, looks like a simple update:
WITH cte AS
( SELECT -- do all the counts using conditional aggregation
ModuleID
,Count(DISTINCT ID) AS Started
,Count(DISTINCT CASE WHEN ModuleDatecomplete <> '' THEN ID END) AS Completed
,Count(DISTINCT CASE WHEN Pass = 'Yes' THEN ID END) AS Passed
FROM TblResults
GROUP BY ModuleID
)
UPDATE TblModules
FROM cte
SET
Started = cte.Started
,Completed = cte.Completed
,Passed = cte.Passed
,PassedPercent = ((cte.Passed * 100.0) / cte.Started)
WHERE
ModuleID = cte.ModuleID
Of course this is fully untested, but the syntax should be ok as-is.
Edit:
In 99+% rewriting a cursor with set-based SQL improves performance.
In your case you run three Selects and one Update per row, e.g. for 100 rows returned by the cursor Select the DBMS needs to execute 400 statements sequentially, 300 Selects and 100 Updates. Combining the three SETs into one using conditional aggregation would reduce the amount of work to 100 Selects and 100 Updates and result in approx. half the runtime. But a single Update might run 10 or 100 or 1000 times faster.
-
\$\begingroup\$ I know this is an old answer, but could you please add a little more explanation why this works as good or better? \$\endgroup\$Malachi– Malachi2018年07月04日 18:21:46 +00:00Commented Jul 4, 2018 at 18:21