How to reduce the run time of stored procedure by getting rid of cursors?

Question

I have a task of reducing the run time of a stored procedure that runs for hours.

Description of task: The cursor takes a set of values @Primary and @Secondary obtained from a query(5000+ rows), finds the @isNewProgram and @NewProgram through the value of @Primary and then operates upon another inbuilt cursor within which the values of each row of the 5000 is compared against some condition and inserted into @tempTable finally. but the individual non parallel processing of each row at a time with cursors is consuming a large time.

I have optimized everything from indexes to usage of common table expressions but how far I try I cannot even start to remove cursors and replace it with set based approach.

SET @CUR_PSD = CURSOR FOR
SELECT DISTINCT B.Primary,Secondary 
FROM DetailsTable A 
Inner join GroupTable B on B.Primary = A.Primary
WHERE CurrentMonth  = @CurrentMonth AND (LEN(Primary) <> 6)

SET @Cur_Billable = CURSOR FOR
SELECT DISTINCT Liability FROM @Liability

OPEN @CUR_PSD
    
FETCH NEXT FROM @CUR_PSD INTO @Primary,@Secondary
                
IF @@FETCH_STATUS <> 0 
    Print 'Error in the Cursor Fetch Statement'
            
WHILE @@FETCH_STATUS = 0    

BEGIN
    SELECT @IsNewProgram = (SELECT Distinct IsNewProgram 
                    FROM GroupTable WHERE Primary = @Primary)
    SELECT @NewProgram = (SELECT Distinct NewProgram
                    FROM GroupTable WHERE Primary = @Primary)
    
        
    OPEN @Cur_Billable
    
    FETCH NEXT FROM @Cur_Billable INTO @VarLiability
                    
    IF @@FETCH_STATUS <> 0 
        Print 'Error in the Cursor Fetch Statement'
                
    WHILE @@FETCH_STATUS = 0    
    
    BEGIN
    
    INSERT INTO @TempTable
    SELECT top 10 Secondary, Primary, GroupName,Liability, DefectCount, DefectCode
        FROM (
                SELECT  Secondary
                        ,@Primary AS Primary
                        ,GroupName
                        ,Liability
                        ,SUM(DefectCount) AS DefectCount,   
                        DefectCode
                FROM @PDTopTable
                WHERE Liability = @VarLiability                         
                
                and TPPrimary IN (SELECT Primary FROM GroupTable C 
                            WHERE C.Primary = @Primary)
                AND  TPNewProgram = CASE  WHEN @IsNewProgram = 0 
                                                Then  @NewProgram           
                                    WHEN @IsNewProgram = 2 
                                                Then TPNewProgram
                                ELSE  @NewProgram  END  
                AND TPSecondary = @Secondary            
                GROUP BY Secondary, GroupName, DefectCode,Liability     
        UNION
                SELECT  Secondary
                        ,@Primary AS Primary
                        ,GroupName
                        ,Liability
                        ,SUM(DefectCount) AS DefectCount,   
                        DefectCode
                FROM @PDBotTable
                WHERE Liability = @VarLiability                         
                
                and TPPrimary IN (SELECT Primary FROM GroupTable C 
                            WHERE C.Primary = @Primary)
                AND  TPNewProgram = CASE  WHEN @IsNewProgram = 0 
                                            Then  @NewProgram           
                                    WHEN @IsNewProgram = 2 
                                            Then TPNewProgram
                                ELSE  @NewProgram  END  
                AND TPSecondary = @Secondary            
                GROUP BY Secondary, GroupName, DefectCode,Liability
        ) TDG
        ORDER BY  TDG.Primary, TDG.Secondary,TDG.DefectCount DESC
    
        FETCH NEXT FROM @Cur_Billable INTO @VarLiability
    
        END 

        CLOSE @Cur_Billable
    
    FETCH NEXT FROM @CUR_PSD INTO @Primary,@Secondary
    
END 

CLOSE @CUR_PSD

I am not looking for someone to do the conversion for me, I just need to know how to get started. I've found a lot of tutorials but I need the right guidance to get started with this as I have never done this before.

Just so you people get to know the outer cursor operates on 5000 rows or more and that condition checking by the outer cursor is what that is making the code run so slow. Should i use inner join on it so that the same desired output can be obtained...?

I mean i want to know what should i do with

 With TQ_PlantSupplierDefects(PlantCode,SupplierCode) AS
(
SELECT DISTINCT B.PrimaryCode as PlantCode,SupplierCode 
    FROM [SMS].[T_SMS_PlantDetailDefects] A 
    Inner join SMS.T_SMS_PlantGroup B on B.PlantCode = A.PlantCode
    WHERE RollingYearMonth  = @RollingYearMonth AND (LEN(PrimaryCode) <> 6)
)

If i am going to operate upon the above mentioned CTE and @PDTopTable what should i use. My instinct says i should use combination of functions and joins. Am i right?

Answer 1

You want to increase the speed of your cursors? Wrap them in a transaction. If you are processing millions of records and don't want/need them all in one transaction, you can commit it on occasion to reduce resources.

I did this with a cursor that took an hour to run (this is an extreme case) and afterword it ran in 1 1/2 minutes.

I know that does not answer you question but it might help you avoid doing the conversion until your more familiar with SQL. The answer to your question is... experience. There is no magic site or book you can read, it just takes time learning to do more and more in a single statement.

Answer 2

If I'm following correctly, the general purpose of the code is:

For every valid primary, secondary, and liability, insert up to 10 records into @TempTable (based on the values of IsNewProgram and NewProgram).

First: If there can be more than one record in GroupTable for each Primary, then your @IsNewProgram and @NewProgram values could be any of the possible values for that Primary's rows. If that's the intention, you could set those in the outer cursor (GROUP BY Primary, Secondary, and include IsNewProgram and NewProgram as a MAX or MIN, to ensure a valid value). This removes the need to set those for each Primary and Secondary in the loop.

Second: Unless a Primary could be removed from GroupTable during the execution of the query, checking the following might not be required:

          and TPMPrimary IN (SELECT Primary FROM GroupTable C 
                        WHERE C.Primary = @Primary)

If we can successfully pull everything with a single query, then this will definitely not be needed. If, however, we're using multiple queries, it may still be worth confirming. If other changes don't provide enough performance boost, try taking the check out, and see if it actually makes a difference or not.

Last: You might be able to avoid the cursors by using ROW_NUMBER(). You'd keep the TDG subquery basically as is (modulo the steps above, and expanded to include all primaries, secondaries, and liabilities at once). To the outer query, instead of TOP 10, you'd add a new column:

    ,ROW_NUMBER() OVER (PARTITION BY Primary, Secondary, Liability ORDER BY DefectCount DESC) as row_no

Next, make this SQL statement into a subquery of another SQL statement. In the new outer SELECT, grab all the columns except row_no, and use the following WHERE clause:

WHERE row_no <= 10

Fundamentally, you need to identify what's forcing you to use a cursor (pulling up IsNewProgram and NewProgram within the loop; and, needing the top ten results for each primary, secondary, and liability), and look for options that let you accomplish the same things in a set-based way.

NOTE: Not rigorously tested, as you asked for more of a pointer than a specific solution.