Bigeyes
Bigeyes

Reputation: 1666

Cursor poor performance issue

I have a query which runs very slow. After some observations, I feel that it is because of using cursor in the query.

CREATE PROCEDURE [dbo].[spTest] (@INwww  varchar(6))  AS

declare @curwwwbegdate datetime
declare @prevwwwbegdate datetime
declare @PrevwwwNum varchar(6)
set NOCOUNT ON
set @CurwwwBegDate = (select Begindate from wwwNUM where wwwnumber = @Inwww)
set @PrevwwwNum = (SELECT TOP 1 wwwNUMBER from wwwNUM WHERE BEGINDATE < @curwwwbegdate
order by begindate desc)
CREATE TABLE #ContRevExp (Inum int, lwww varchar(6),BookNum varchar(13), ldate datetime, lllLoc varchar(8), currentrev varchar(20), currentexp varchar(20), oldrev varchar(20), oldexp varchar(20), ContFileLoc varchar(100), bookloc varchar(3))
create table #wwwdates (wdLoc varchar(3),wdwww varchar(6), wdBegDate datetime, wdEndDate datetime)
create table #LocTable (loc varchar(3))
insert into #LocTable
SELECT LocCode from Location WHERE  STATUS = 1 AND CCC = 1

DECLARE LocCursor INSENSITIVE CURSOR FOR
select * from #LocTable
OPEN LocCursor
declare @Loc varchar(3)
FETCH NEXT FROM LocCursor into @Loc
WHILE (@@FETCH_STATUS = 0)
BEGIN
insert into #wwwdates (wdLoc, wdwww, wdBegDate, wdEndDate)
 select @Loc,@Inwww,
(select top 1 wwwllldate from cccswwwllled where lllwww = @PrevwwwNum and branch = @Loc),
(select  top 1 wwwllldate from cccswwwllled where lllwww = @Inwww and branch = @Loc)

FETCH NEXT FROM LocCursor into @Loc
END
deallocate LocCursor
Drop table #LocTable

DECLARE LocwwwCursor INSENSITIVE CURSOR FOR
select wdLoc, wdBegDate, wdEndDate from #wwwdates
declare @INFILELOC as varchar(3)
declare @INBEGDATE  datetime
declare @INENDDATE  datetime
OPEN LocwwwCursor
FETCH NEXT FROM LocwwwCursor into @INFILELOC,  @INBEGDATE, @INENDDATE
WHILE (@@FETCH_STATUS = 0)
BEGIN

declare @INVNUM int
declare @INDATE datetime

create table #cccs (cInvNum int)
insert into #cccs
Select distinct cccInv_Num from cccswwwllled
left join cccsexport on cccsExport.inv_num = cccinv_num
where cccswwwllled.Branch = @INFILELOC
and (cccswwwllled.impexp <> 1 or cccswwwllled.impexp is null)
and lllwww < @INwww and ContBookIndicator = 'C'
and cccsexport.lastupdate >= @INBEGDATE and cccsexport.lastupdate < @INENDDATE 
DECLARE cccCursor INSENSITIVE CURSOR FOR
select * from #cccs

OPEN cccCursor
declare @cnum int
FETCH NEXT FROM cccCursor into @cnum
WHILE (@@FETCH_STATUS = 0)
BEGIN
insert into #ContRevExp (Inum , lwww ,BookNum, ldate , lllloc , currentrev , currentexp, oldrev, oldexp, contfileloc, bookloc )

select top 1 cccInv_Num, lllwww,
(Select top 1 cccNumber from cccsExport where inv_num = @cnum and lastupdate <= @INENDDATE order by lastupdate desc ),
wwwlllDate,CE.FileNum,
0,
--case when CE.STATUS = 0 then '0' else convert(varchar(20),CE.TotalCost + CE.AllinRpoCost) end,
case when CE.STATUS = 0 then '0' else convert(varchar(20),CE.TotalExpense) end,
0,
--(Select top 1 convert(varchar(20),TotalCost + Allinrpocost) from cccsExport where inv_num = @cnum and lastupdate <= @INBEGDATE order by lastupdate desc ),
(Select top 1 convert(varchar(20),TotalExpense) from cccsExport where inv_num = @cnum and lastupdate <= @INBEGDATE order by lastupdate desc ),
'Cont/File/Loc',
BookLocation from cccswwwllled as CWL
left join cccsexport as CE on CE.inv_num = CWL.cccInv_Num
where CWL.cccInv_Num = @cnum 
and CWL.lllwww < @INwww and CWL.ContBookIndicator = 'C'
and CE.lastupdate >= @INBEGDATE and CE.lastupdate <= @INENDDATE
order by CE.lastupdate desc
FETCH NEXT FROM cccCursor into @cnum
END
deallocate cccCursor
drop table #cccs

FETCH NEXT FROM LocwwwCursor into @INFILELOC,  @INBEGDATE, @INENDDATE
END
deallocate LocwwwCursor
drop table #wwwdates

select bookloc,lwww,booknum,Inum,oldrev,oldexp,currentrev,currentexp,lllloc,
case when contfileloc is null then 'NOT' ELSE contfileloc end as 'Cont' from #ContRevExp
where (currentrev <> oldrev) or (currentexp <> oldexp)
order by bookloc, booknum
drop table #ContRevExp

I am not strong on cursor. My question is how to modify the code to improve the performance?

EDIT

The real problem is that I use ado.net to retrieve the data in a web application, the entire time to download data from server to the client is about 10 seconds, By using telerik justtrace tool, I found the stored procedure spent 99% time.

In the query, I found there is a nest cursors usage. cccCursor and LocwwwCursor. It means that there are two while loops, one is inside the other one. I guess that is the main reason, so I hope to replace the cursors with other skills.

SAMPLE DATA:

Because there are a lot of tables. I try to make the logic clear. We get the data from table Location:

The temp table #LocTable only has one column. The sample data is

AAA
BBB
CCC
DDD
EEE
FFF
GGG

I believe LocCursor is dealing each value of above data.
The input of the stored procedure is Inwww, let say it is 200218. We have the query set @CurwwwBegDate = (select Begindate from wwwNUM where wwwnumber = @Inwww) will return a datetime 2002-04-28 00:00:00.000.

Another query set @PrevwwwNum = (SELECT TOP 1 wwwNUMBER from wwwNUM WHERE BEGINDATE < @curwwwbegdate order by begindate desc) will return 200217. The temp table #wwwdates has the sample data as below.

wdLoc   wdWeek  wdBegDate                      wdEndDate
AAA     200218  NULL                           NULL
BBB     200218  NULL                           NULL
CCC     200218  NULL                           NULL
DDD     200218  2002-05-06 13:39:31.000        2002-05-07 16:52:44.000
EEE     200218  NULL                           NULL
FFF     200218  2002-05-06 13:39:40.000        2002-05-07 16:53:42.000
GGG     200218  NULL                           NULL

I think that the cursor LocwwwCursor takes the value from the temp table #wwwdatae then insert #cccs. I haven't got the sample data yet.

The final result likes(I just demonstrate it with one row)

ARL ARL 200510  IETARL0510087   150547816   $0.00   155.21  $0.00   155.00  ARL SOMETHING

Upvotes: 0

Views: 282

Answers (2)

Ross Bush
Ross Bush

Reputation: 15175

There are unnecessary loops for aggerates in the SP above. Most of your logic could be condensed and made more performant with grouping and aggregations. For example, The whole block below could be drastically simplified.

REPLACE THIS ....

declare @prevwwwbegdate datetime
declare @PrevwwwNum varchar(6)
set NOCOUNT ON
set @CurwwwBegDate = (select Begindate from wwwNUM where wwwnumber = @Inwww)
set @PrevwwwNum = (SELECT TOP 1 wwwNUMBER from wwwNUM WHERE BEGINDATE < @curwwwbegdate
order by begindate desc)
CREATE TABLE #ContRevExp (Inum int, lwww varchar(6),BookNum varchar(13), ldate datetime, lllLoc varchar(8), currentrev varchar(20), currentexp varchar(20), oldrev varchar(20), oldexp varchar(20), ContFileLoc varchar(100), bookloc varchar(3))
create table #wwwdates (wdLoc varchar(3),wdwww varchar(6), wdBegDate datetime, wdEndDate datetime)
create table #LocTable (loc varchar(3))
insert into #LocTable
SELECT LocCode from Location WHERE  STATUS = 1 AND CCC = 1

DECLARE LocCursor INSENSITIVE CURSOR FOR
select * from #LocTable
OPEN LocCursor
declare @Loc varchar(3)
FETCH NEXT FROM LocCursor into @Loc
WHILE (@@FETCH_STATUS = 0)
BEGIN
insert into #wwwdates (wdLoc, wdwww, wdBegDate, wdEndDate)
 select @Loc,@Inwww,
(select top 1 wwwllldate from cccswwwllled where lllwww = @PrevwwwNum and branch = @Loc),
(select  top 1 wwwllldate from cccswwwllled where lllwww = @Inwww and branch = @Loc)

FETCH NEXT FROM LocCursor into @Loc
END
deallocate LocCursor
Drop table #LocTable

WITH THIS ....

create table #wwwdates (wdLoc varchar(3),wdwww varchar(6), wdBegDate datetime, wdEndDate datetime)
insert into #wwwdates 
SELECT
    LocCode,@Inwww,MAX(Prev_MaxDate),MAX(In_MaxDate)
FROM
(
    SELECT LocCode,
        Prev_MaxDate=CASE WHEN cccswwwllled.lllwww=@PrevwwwNum THEN wwwllldate ELSE NULL END,
        In_MaxDate=CASE WHEN cccswwwllled.lllwww=@Inwww THEN wwwllldate ELSE NULL END
    from 
        Location 
        INNER JOIN cccswwwllled ON cccswwwllled.lllwww IN(@PrevwwwNum,@Inwww) AND cccswwwllled.Branch=Location.LocCode
    WHERE Location.STATUS = 1 AND Location.CCC = 1
)AS X
GROUP BY
    LocCode

Upvotes: 2

Deadsheep39
Deadsheep39

Reputation: 611

You should remove cursors and create set-based solution. You and mssql engine are not strong in cursor solutions :).

  1. Rething what you would like to do. Join wwwNUM + lllwww + Location into view something like + check indexes on wwwnumber, branch, LocCode + STATUS + CCC (depends on data types)

    create view MyViewWithNameWhichDescriptContain
    as
    select 
        l.LocCode,
        n.wwwNUMBER Inwww,
        n.Begindate InwwwDate,
        c.wwwllldate Inwww
    from Location l, wwwNUM n, cccswwwllled c
    where l.[STATUS] = 1 AND l.CCC = 1
        and l.LocCode = c.branch
        and c.lllwww =  n.wwwnumber
    
  2. select from this view:

      if object_id('tempdb..#wwwMyDatesPerLocCode') is not null drop table #wwwMyDatesPerLocCode
      select *
      into #wwwMyDatesPerLocCode
      from (
           select *, row_number() over(partition by Inwww order by InwwwDate desc OrbyCol
           from MyViewWithNameWhichDescriptContain) t
      where OrbyCol in (1,2)
    
  3. create view cccswwwllled + cccsexport lets say llledExported

  4. make selfjoin to access descendant (wwwMyDatesPerLocCode where OrbyCol = 1 + wwwMyDatesPerLocCode where OrbyCol = 2) and connect with view llledExported to get your info with your filters
  5. check indexes on all tables again

Finnaly you don't need variables and cursors. Try think in joins. If you will add some somple data from all tables and add description of your indexes I can make it more detailed.

Upvotes: 0

Related Questions