views:

68

answers:

2

Can anyone give me a hand improving the performance of this cursor logic from SQL 2000. It runs great in SQl2005 and SQL2008, but takes at least 20 minutes to run in SQL 2000. BTW, I would never choose to use a cursor, and I didn't write this code, just trying to get it to run faster. Upgrading this client to 2005/2008 is not an option in the immediate future.

-------------------------------------------------------------------------------
------- Rollup totals in the chart of accounts hierarchy
-------------------------------------------------------------------------------
DECLARE @B_SubTotalAccountID int, @B_Debits money, @B_Credits money, @B_YTDDebits money, @B_YTDCredits money

DECLARE Bal CURSOR FAST_FORWARD FOR 
 SELECT SubTotalAccountID, Debits, Credits, YTDDebits, YTDCredits FROM xxx 
  WHERE AccountType = 0 AND SubTotalAccountID Is Not Null and (abs(credits)+abs(debits)+abs(ytdcredits)+abs(ytddebits)<>0)
OPEN Bal

FETCH NEXT FROM Bal INTO @B_SubTotalAccountID, @B_Debits, @B_Credits, @B_YTDDebits, @B_YTDCredits

--For Each Active Account
WHILE @@FETCH_STATUS = 0
 BEGIN

 --Loop Until end of subtotal chain is reached
 WHILE @B_SubTotalAccountID Is Not Null
  BEGIN

  UPDATE xxx2
  SET Debits = Debits + @B_Debits,
   Credits = Credits + @B_Credits,
   YTDDebits = YTDDebits + @B_YTDDebits,
   YTDCredits = YTDCredits + @B_YTDCredits
  WHERE GLAccountID = @B_SubTotalAccountID

  SET @B_SubTotalAccountID = (SELECT SubTotalAccountID FROM xxx2 WHERE GLAccountID = @B_SubTotalAccountID)

  END

 FETCH NEXT FROM Bal INTO @B_SubTotalAccountID, @B_Debits, @B_Credits, @B_YTDDebits, @B_YTDCredits

 END

CLOSE Bal
DEALLOCATE Bal
+1  A: 

A couple of suggestions:

1 - use the profiler to tell you which part of it runs slowly - you can get a duration for each statement

2 - run the initial select statement (the cursor declaration) outside of the procedure and check the query plan. Does it run quickly? Is it using indexes properly?

3 - same thing with the update statement - check the query plan and index usage

4 - the 'set' statement after the update looks odd - it seems to be getting a value into @B_SubTotalAccountID which is then replaced immediately by the 'fetch next'

Ray
+3  A: 
Update xx2
Set Credits = Credits + X1.CreditTotal
    , Debits = Debits + X1.DebitTotal
    , YtdDebits = YtdDebits + X1.YtdDebitTotal
    , YtdCredits = YtdCredits + X1.YtdDebitTotal
From xx2 As X2
    Join    (
            Select SubTotalAccountID, Sum(Debits) As DebitTotal, Sum(Credits) As CreditTotal
                , Sum(YtdDebits) As YtdDebitTotal, Sum(YtdCredits) As YtdCreditTotal
            From xxx
            Where AccountType = 0 
                And SubTotalAccountID Is Not Null 
                And (
                    Credits <> 0
                    Or Debits <> 0
                    Or YtdCredits <> 0
                    Or YtdDebits <> 0
                    )
            Group By SubTotalAccountID
            ) As X1
        On X1.SubTotalAccountID = X2.GLAccountID

Without schema, I could not tell if the xxx table would return multiple rows for a given SubTotalAccountId. I assumed that it could and grouped the values by this column so that I get one row per SubTotalAccountId.

I also replaced your use of ABS in the WHERE clause with simply checks against zero. This should be substantially faster.

This UPDATE statement should be a complete replacement for your cursor.

Thomas
I always prefer flat updates to Cursors. Cursors are evil.Sometimes necessary, but still evil...
AllenG