January 13, 2010 at 12:06 pm
I've read numerous articles telling us to avoid using cursors wherever possible in favour of while loops, however, is there a point where a cursor becomes more efficient than a while loop when working with large result sets.
I have 4 SPs that both use 2 cursors that I inherited and thought they were prime candidates for a performance increase by replacing with a WHILE loop.
On testing, the performance with the WHILE loop is painfully slow compared to the code using the cursor.
There are 151000+ rows in the data set and the cursor is loads faster than the while loop, unless there's something wrong with my new code.
Old and new code attached for reference.
Any ides?
ORIGINAL CODE
DECLARE @MinNumValsToCalcTAPAS INTEGER
SET@MinNumValsToCalcTAP=10
DECLARE@CSD_AIMSNoAS CHAR(5)
DECLARE @LoanType_NewGroup AS CHAR(3)
DECLARE @CSD_NomIPTBLBAS CHAR(4)
DECLARE @CSD_LineValueInclVAT AS FLOAT
DECLARE @CSD_MIN_LineValueInclVAT AS FLOAT
DECLARE @TAPID AS INTEGER
DECLARE @Cttr AS CHAR(5)
DECLARE @LoanType AS CHAR(3)
DECLARE @IPTAS CHAR(4)
-- Create Cursor for CSD MAX TAP01 ID based on the ascending Line Value Grouped by Cttr+Loan+IPT --
DECLAREcurCSDOrderedByCttrLoanIPTMAXValue
cursor for
SELECTCSD_AIMSNo, LoanType_NewGroup, CSD_NomIPTBLB, CSD_LineValueInclVAT
FROMCSD
INNER JOINLoanType
ONCSD_LoanType=LoanType_Code
WHERECSD_LineValueInclVAT > 0
AND CSD_ClosingBalance>0
--ANDISNULL(CSD_AssetCategory,'')<>'BBC'
ANDISNULL(CSD_AssetCategory,'')in('DAF','DFF','BWD')
ANDCSD_LoanType<>9
ORDER BYCSD_AIMSNo, LoanType_NewGroup, CSD_NomIPTBLB, CSD_LineValueInclVAT
-- Calculate the MAX TAP01 ID based on the ascending Line Value Grouped by Cttr+Loan+Domain --
OPENcurCSDOrderedByCttrLoanIPTMAXValue
FETCH NEXTFROM curCSDOrderedByCttrLoanIPTMAXValue
INTO @CSD_AIMSNo, @LoanType_NewGroup, @CSD_NomIPTBLB, @CSD_LineValueInclVAT
--Initialise Variables--
SET@TAPID = 1
SET@Cttr = '0'
SET@LoanType = 'ZED'
SET@IPT = '&'
--Iterate Until End Of Cursor File--
WHILE @@fetch_status = 0
begin
IF NOT((@IPT=@CSD_NomIPTBLB)
AND(@LoanType=@LoanType_NewGroup)
AND (@Cttr=@CSD_AIMSNo)
)
BEGIN
SET@TAPID = 1
END
ELSE
BEGIN
SET @TAPID = @TAPID + 1
END
--Update the applicable TAP ID--
UPDATECSD
SETCSD_TAP1_ID = @TAPID WHERE CURRENT OF curCSDOrderedByCttrLoanIPTMAXValue
SET@IPT = @CSD_NomIPTBLB
SET@LoanType = @LoanType_NewGroup
SET@Cttr = @CSD_AIMSNo
FETCH NEXTFROM curCSDOrderedByCttrLoanIPTMAXValue
INTO @CSD_AIMSNo, @LoanType_NewGroup, @CSD_NomIPTBLB, @CSD_LineValueInclVAT
END
--Remove Cursor Definition to reduce memory usage--
DEALLOCATEcurCSDOrderedByCttrLoanIPTMAXValue
NEW CODE
DECLARE @MinNumValsToCalcTAPAS INTEGER
SET@MinNumValsToCalcTAP=10
DECLARE @TAPID AS INTEGER
DECLARE @Cttr AS CHAR(5)
DECLARE @LoanType AS CHAR(3)
DECLARE @IPTAS CHAR(4)
DECLARE @NumberRecords int, @RowCount int
DECLARE @NumberRecordsMIN int, @RowCountMIN int
DECLARE @CSD_ID int, @CSD_AIMSNo char(5), @LoanType_NewGroup char(3), @CSD_NomIPTBLB char(4), @CSD_LineValueInclVAT float, @CSD_MIN_LineValueInclVAT float
CREATE TABLE #CSDOrderedByCttrLoanIPTMAXValue (
RowID int IDENTITY(1,1),
CSD_ID int,
CSD_AIMSNo char(5),
LoanType_NewGroup char(3),
CSD_NomIPTBLB char(4),
CSD_LineValueInclVAT float
)
-- Insert the resultset we want to loop through
-- into the temporary table
INSERT INTO #CSDOrderedByCttrLoanIPTMAXValue (CSD_ID, CSD_AIMSNo, LoanType_NewGroup, CSD_NomIPTBLB, CSD_LineValueInclVAT)
SELECT CSD_ID, CSD_AIMSNo, LoanType_NewGroup, CSD_NomIPTBLB, CSD_LineValueInclVAT
FROM CSD
INNER JOINLoanType
ONCSD_LoanType=LoanType_Code
WHERECSD_LineValueInclVAT > 0
AND CSD_ClosingBalance>0
--ANDISNULL(CSD_AssetCategory,'')<>'BBC'
ANDISNULL(CSD_AssetCategory,'')in('DAF','DFF','BWD')
ANDCSD_LoanType<>9
ORDER BYCSD_AIMSNo, LoanType_NewGroup, CSD_NomIPTBLB, CSD_LineValueInclVAT
-- Get the number of records in the temporary table
SET @NumberRecords = @@ROWCOUNT
SET @RowCount = 1
--Initialise Variables--
SET@TAPID = 1
SET@Cttr = '0'
SET@LoanType = 'ZED'
SET@IPT = '&'
-- loop through all records in the temporary table
-- using the WHILE loop construct
WHILE @RowCount <= @NumberRecords
BEGIN
SELECT @CSD_ID = CSD_ID, @CSD_AIMSNo = CSD_AIMSNo, @LoanType_NewGroup = LoanType_NewGroup,
@CSD_NomIPTBLB = CSD_NomIPTBLB, @CSD_LineValueInclVAT = CSD_LineValueInclVAT
FROM #CSDOrderedByCttrLoanIPTMAXValue
WHERE RowID = @RowCount
IF NOT((@IPT=@CSD_NomIPTBLB)
AND(@LoanType=@LoanType_NewGroup)
AND (@Cttr=@CSD_AIMSNo)
)
BEGIN
SET@TAPID = 1
END
ELSE
BEGIN
SET @TAPID = @TAPID + 1
END
--Update the applicable TAP ID--
UPDATECSD
SETCSD_TAP1_ID = @TAPID
WHERE CSD_ID = @CSD_ID
SET@IPT = @CSD_NomIPTBLB
SET@LoanType = @LoanType_NewGroup
SET@Cttr = @CSD_AIMSNo
SET @RowCount = @RowCount + 1
END
January 13, 2010 at 12:31 pm
It looks to me like all this does is create an ascending number partitioned by certain columns in the data set. Is that correct?
If so, then SQL 2000 doesn't offer a lot of good options on that. You could replace it with a "quirky update" and probably improve performance, at the cost of the usual issues with that technique.
Personally, in SQL 2000, I'd use a cursor for this. I'd declare it as updateable, and use it to update the actual rows, since that gives you better transactional isolation than the current script. If you want to keep it fast at the cost of transactional integrity, then I'd declare the cursor as local and fast_forward to speed it up even more.
Of course, in SQL 2005, you could replace all of this with a simple CTE and use of the Row_Number function, and do it all in one go. That would be better, but you posted in the SQL 2000 forum, so I'm assuming that's what you're using.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
January 13, 2010 at 1:16 pm
That's exactly what it does and utilises that value in a later stage of the SP (which I didn't include in the code).
The while loop is horrendously slow compared to the cursor and yes our live DB is running on 2000.
How would I write the cursor with the other features that you mention?
I'm not that familiar with cursors.
Thanks for the quick reply.
Andrew
January 13, 2010 at 2:46 pm
Change:
DECLARE curCSDOrderedByCttrLoanIPTMAXValue
cursor for
to
DECLARE curCSDOrderedByCttrLoanIPTMAXValue
cursor local fast_forward for
You're just adding the two tags to the declare statement.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
Viewing 4 posts - 1 through 4 (of 4 total)
You must be logged in to reply to this topic. Login to reply