0

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 @Master and @Slave obtained from a query(5000+ rows), finds the @isNewProgram and @NewProgram through the value of @Master 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.Master,Slave  
FROM DetailsTable A 
Inner join GroupTable B on B.Master = A.Master
WHERE CurrentMonth  = @CurrentMonth AND (LEN(Master) <> 6)

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

OPEN @CUR_PSD

FETCH NEXT FROM @CUR_PSD INTO @Master,@Slave

IF @@FETCH_STATUS <> 0 
    Print 'Error in the Cursor Fetch Statement'

WHILE @@FETCH_STATUS = 0    

BEGIN
    SELECT @IsNewProgram = (SELECT Distinct IsNewProgram 
                    FROM GroupTable WHERE Master = @Master)
    SELECT @NewProgram = (SELECT Distinct NewProgram
                    FROM GroupTable WHERE Master = @Master)


    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 Slave, Master, GroupName,Liability, DefectCount, DefectCode
        FROM (
                SELECT  Slave
                        ,@Master AS Master
                        ,GroupName
                        ,Liability
                        ,SUM(DefectCount) AS DefectCount,   
                        DefectCode
                FROM @PDTopTable
                WHERE Liability = @VarLiability                         

                and TPMaster IN (SELECT Master FROM GroupTable C 
                            WHERE C.Master = @Master)
                AND  TPNewProgram = CASE  WHEN @IsNewProgram = 0 
                                                Then  @NewProgram           
                                    WHEN @IsNewProgram = 2 
                                                Then TPNewProgram
                                ELSE  @NewProgram  END  
                AND TPSlave = @Slave            
                GROUP BY Slave, GroupName, DefectCode,Liability     
        UNION
                SELECT  Slave
                        ,@Master AS Master
                        ,GroupName
                        ,Liability
                        ,SUM(DefectCount) AS DefectCount,   
                        DefectCode
                FROM @PDBotTable
                WHERE Liability = @VarLiability                         

                and TPMaster IN (SELECT Master FROM GroupTable C 
                            WHERE C.Master = @Master)
                AND  TPNewProgram = CASE  WHEN @IsNewProgram = 0 
                                            Then  @NewProgram           
                                    WHEN @IsNewProgram = 2 
                                            Then TPNewProgram
                                ELSE  @NewProgram  END  
                AND TPSlave = @Slave                
                GROUP BY Slave, GroupName, DefectCode,Liability
        ) TDG
        ORDER BY  TDG.Master, TDG.Slave,TDG.DefectCount DESC

        FETCH NEXT FROM @Cur_Billable INTO @VarLiability

        END 

        CLOSE @Cur_Billable

    FETCH NEXT FROM @CUR_PSD INTO @Master,@Slave

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.MasterCode 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(MasterCode) <> 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?

  • How many rows get processed during those hours? How heavy a load does this put on the server? Note: For very large data sets it is a good idea to process separate batches of rows. – RLF Aug 8 '14 at 18:29
  • Yes it is a large table with a lot of rows. But that isn't the problem i minimized the fetch time using indexes and all. But the recursive cursor is the one that elongates the run time the outer cursor processes upon like 5000 values and there are like 5 similar cursors within the same stored procedure. Note: Also can you tell me how to process in separate batch of rows – Riddler Aug 9 '14 at 4:37
  • 1
    A description of what the code is intended to do will help immensely with understanding the existing code. – Pieter Geerkens Aug 9 '14 at 4:46
  • For dividing an update into batches, see the SQL Server Pro posting from 2003: sqlmag.com/database-performance-tuning/dividing-update-batches – RLF Aug 11 '14 at 17:37
2

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.

  • Thanks for your input Steve. I am trying your method and will test it once i get to the database and run it. – Riddler Aug 9 '14 at 4:48
  • You can also declare the cursor LOCAL FORWARD_ONLY to help speed up the cursor a little bit. – Danielle Paquette-Harvey Dec 5 '17 at 16:39
0

(Just before posting, I noticed that the question is about 2 1/2 years old now. So, the OP presumably doesn't need this; however, since it popped up as an "unanswered" question, thought I may as well go ahead and post anyway).

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

For every valid master, slave, 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 Master, then your @IsNewProgram and @NewProgram values could be any of the possible values for that Master's rows. If that's the intention, you could set those in the outer cursor (GROUP BY Master, Slave, and include IsNewProgram and NewProgram as a MAX or MIN, to ensure a valid value). This removes the need to set those for each Master and Slave in the loop.

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

          and TPMaster IN (SELECT Master FROM GroupTable C 
                        WHERE C.Master = @Master)

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 masters, slaves, and liabilities at once). To the outer query, instead of TOP 10, you'd add a new column:

    ,ROW_NUMBER() OVER (PARTITION BY Master, Slave, 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 master, slave, 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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.