February 4, 2013 at 10:56 pm
I have a function with cursors which returns a table in SQL Server 2008. On executing the query which calls this function, it takes more than 2 hours to complete. Can anybody tell me how to optimize this function or where is the problem. I checked it with SQL profiler, but couldn't find any solution. The above mentioned function is below:
Returns
@TAB_STOCK Table (WH NVARCHAR(20),ITEMGROUPNAME NVARCHAR(100),ITEMCODE NVARCHAR(20), ITEMDESC NVARCHAR(100), OB NUMERIC(16,4), OB_VAL NUMERIC(16,4), IN_QTY NUMERIC(16,4), IN_VAL NUMERIC(16,4), ITMGRP_IN_QTY NUMERIC(16,4), ITMGRP_IN_VAL NUMERIC(16,4), OUT_QTY NUMERIC(16,4), OUT_VAL NUMERIC(16,4), ITMGRP_OUT_QTY NUMERIC(16,4), ITMGRP_OUT_VAL NUMERIC(16,4), CB_QTY NUMERIC(16,4), CB_VAL NUMERIC(16,4),packcartons NUMERIC(16,4),itmprice NUMERIC(16,4),ITMGRPCD NVARCHAR(20),DTE DATETIME)
AS
BEGIN
--Declaration Part
DECLARE @WAREHOUSE NVARCHAR(20), @ITEMCODE NVARCHAR(20), @ITEMDESC NVARCHAR(100), @ITEMGROUP NVARCHAR(20), @OPENQTY NUMERIC(16,4), @OPENVALUE NUMERIC(16,4), @RECEIPTQTY NUMERIC(16,4), @RECEIPTVALUE NUMERIC(16,4), @GROUPTOT_RECEIPTQTY NUMERIC(16,4), @GROUPTOT_RECEIPTVALUE NUMERIC(16,4), @ISSUEDQTY NUMERIC(16,4), @ISSUEDVALUE NUMERIC(16,4), @CLOSINGQTY NUMERIC(16,4), @CLOSINGVALUE NUMERIC(16,4), @GROUPTOT_ISSUEDQTY NUMERIC(16,4), @GROUPTOT_ISSUEDVALUE NUMERIC(16,4)
DECLARE @DATE DATETIME
DECLARE @INQTY NUMERIC(16,4), @OUTQTY NUMERIC(16,4), @TRANSTYPE NVARCHAR(6), @CALCPRICE NUMERIC(16,4)
DECLARE @TOT_IN NUMERIC(16,4), @TOT_OUT NUMERIC(16,4), @TOT_IN_PRICE NUMERIC(16,4), @TOT_OUT_PRICE NUMERIC(16,4)
DECLARE @OPENQTYPRICE NUMERIC(16,4), @OpQty NUMERIC(16,4), @OpVal NUMERIC(16,4), @ClQty NUMERIC(16,4), @ClVal NUMERIC(16,4)
DECLARE @PREVDAYCLOSING NUMERIC(16,4), @PREVDAYCLOSINGVAL NUMERIC(16,4), @DAYOPENINGBALANCE NUMERIC(16,4), @DAYOPENINGBALANCEVAL NUMERIC(16,4), @DAYCLOSINGBALANCE NUMERIC(16,4), @DAYCLOSINGBALANCEVAL NUMERIC(16,4)
DECLARE @TAB_DAY_STOCK TABLE (WH NVARCHAR(20),ITMCORTON NUMERIC(16,4), ITEMGROUP NVARCHAR(20),ITEMCODE NVARCHAR(20), DATE DATETIME, OB NUMERIC(16,4), OB_VAL NUMERIC(16,4), IN_QTY NUMERIC(16,4), IN_QTY_VAL NUMERIC(16,4), OUT_QTY NUMERIC(16,4), OUT_QTY_VAL NUMERIC(16,4), CB_QTY NUMERIC(16,4), CB_QTY_VAL NUMERIC(16,4), INV_REV_FLAG INT,itmprice NUMERIC(16,4))
DECLARE @ST_DATE DATETIME, @EN_DATE DATETIME, @IN_DATE DATETIME
DECLARE @ITMCOD NVARCHAR(20)
DECLARE @INV_REVALUE NUMERIC(16,4), @INV_REV_FLAG INT
DECLARE @TOT_BALANCE NUMERIC(16,4), @TOT_TRANS_BALANCE NUMERIC(16,4)
declare @packunit NUMERIC(16,4), @packcartons NUMERIC(16,4),@itmprice NUMERIC(16,4)
SET @packunit = 0
SET @packcartons = 0
SET @TOT_BALANCE = 0
SET @TOT_TRANS_BALANCE = 0
SET @INV_REV_FLAG = 0
SET @ST_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE)
SET @EN_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE DESC)
SET @DATE = @ST_DATE
--Warehouse Cursor
DECLARE CUR_WAREHOUSE CURSOR FOR SELECT WHSCODE FROM OWHS WHERE ((WHSCODE >= @F_WAREHOUSE AND WHSCODE <= @T_WAREHOUSE) OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = '')) ORDER BY WHSCODE
OPEN CUR_WAREHOUSE
FETCH NEXT FROM CUR_WAREHOUSE INTO @WAREHOUSE
WHILE(@@FETCH_STATUS = 0)
BEGIN
--Item Group Cursor
DECLARE CUR_ITEMGROUP CURSOR FOR SELECT ITMSGRPCOD FROM OITB
WHERE ((ITMSGRPCOD = @F_ITEMGROUP AND ITMSGRPCOD <= @T_ITEMGROUP)
OR (@F_ITEMGROUP = ''AND @T_ITEMGROUP = ''))
ORDER BY ITMSGRPCOD
OPEN CUR_ITEMGROUP
FETCH NEXT FROM CUR_ITEMGROUP INTO @ITEMGROUP
WHILE(@@FETCH_STATUS = 0)
BEGIN
--Item Cursor
DECLARE CUR_ITEM CURSOR FOR SELECT DISTINCT T6.ITEMCODE FROM OINM T6 WHERE ((T6.ITEMCODE IN (SELECT T0.[ItemCode] FROM OITM T0 INNER JOIN OITB T1 ON T0.ItmsGrpCod = T1.ItmsGrpCod WHERE T1.ITMSGRPCOD = @ITEMGROUP)) AND (T6.WAREHOUSE = @WAREHOUSE)) ORDER BY T6.ITEMCODE -- AND ((T6.DOCDATE >= @F_DATE AND T6.DOCDATE <= @T_DATE) OR (@F_DATE = '' AND @T_DATE = '') )) ORDER BY T6.ITEMCODE
OPEN CUR_ITEM
FETCH NEXT FROM CUR_ITEM INTO @ITEMCODE
WHILE(@@FETCH_STATUS = 0)
BEGIN
SET @PREVDAYCLOSING = 0
SET @PREVDAYCLOSINGVAL = 0
SET @ST_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE)
SET @EN_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE DESC)
SET @DATE = @ST_DATE
WHILE (@DATE <= @EN_DATE)
BEGIN
SET @INQTY = 0
SET @OUTQTY = 0
SET @CALCPRICE = 0
SET @TOT_IN = 0
SET @TOT_OUT = 0
SET @TOT_IN_PRICE = 0
SET @TOT_OUT_PRICE = 0
SET @OPENQTY = 0
SET @OPENQTYPRICE = 0
SET @DAYOPENINGBALANCE = 0
SET @DAYOPENINGBALANCEVAL = 0
SET @DAYCLOSINGBALANCE = 0
SET @DAYCLOSINGBALANCEVAL = 0
SET @INV_REVALUE = 0
DECLARE CUR_ITEM_IN CURSOR FOR SELECT (CONVERT(NUMERIC(16,4), T6.INQTY)), (CONVERT(NUMERIC(16,4), T6.OUTQTY)), (CONVERT(NUMERIC(16,4), T6.TRANSVALUE)), T6.TRANSTYPE FROM OINM T6 WHERE ((T6.ITEMCODE = @ITEMCODE) AND (T6.WAREHOUSE = @WAREHOUSE) AND (T6.DOCDATE = @DATE) and t6.transtype <> 67)
OPEN CUR_ITEM_IN
FETCH NEXT FROM CUR_ITEM_IN INTO @INQTY, @OUTQTY, @CALCPRICE, @TRANSTYPE
WHILE(@@FETCH_STATUS = 0)
BEGIN
IF @INQTY = 0 AND @OUTQTY = 0
SET @INV_REVALUE = @INV_REVALUE + @CALCPRICE
IF @CALCPRICE < 0
SET @CALCPRICE = @CALCPRICE * (-1)
IF (@INQTY > 0)
BEGIN
SET @TOT_IN = @TOT_IN + @INQTY
SET @TOT_IN_PRICE = @TOT_IN_PRICE + @CALCPRICE
END
IF (@OUTQTY > 0)
BEGIN
SET @TOT_OUT = @TOT_OUT + @OUTQTY
SET @TOT_OUT_PRICE = @TOT_OUT_PRICE + @CALCPRICE
END
FETCH NEXT FROM CUR_ITEM_IN INTO @INQTY, @OUTQTY, @CALCPRICE, @TRANSTYPE
END
CLOSE CUR_ITEM_IN
DEALLOCATE CUR_ITEM_IN
SET @INV_REV_FLAG = 0
IF @OPENQTY != 0 OR @TOT_IN != 0 OR @TOT_OUT != 0 OR @F_DATE = @DATE OR @T_DATE = @DATE OR @INV_REVALUE != 0
BEGIN
IF @INV_REVALUE != 0 AND @TOT_IN = 0 AND @TOT_OUT = 0
SET @INV_REV_FLAG = 1
SET @DAYOPENINGBALANCE = @PREVDAYCLOSING
SET @DAYOPENINGBALANCEVAL = @PREVDAYCLOSINGVAL
SET @DAYCLOSINGBALANCE = @DAYOPENINGBALANCE + (@OPENQTY + @TOT_IN) - @TOT_OUT
SET @DAYCLOSINGBALANCEVAL = @DAYOPENINGBALANCEVAL + (@OPENQTYPRICE + @TOT_IN_PRICE) - @TOT_OUT_PRICE + @INV_REVALUE
INSERT INTO @TAB_DAY_STOCK VALUES(@WAREHOUSE,@packcartons, @ITEMGROUP, @ITEMCODE, @DATE, @DAYOPENINGBALANCE, @DAYOPENINGBALANCEVAL, (@OPENQTY + @TOT_IN), (@OPENQTYPRICE + @TOT_IN_PRICE), @TOT_OUT, @TOT_OUT_PRICE, @DAYCLOSINGBALANCE, @DAYCLOSINGBALANCEVAL, @INV_REV_FLAG,@itmprice)
SET @PREVDAYCLOSING = @DAYCLOSINGBALANCE
SET @PREVDAYCLOSINGVAL = @DAYCLOSINGBALANCEVAL
IF @T_DATE = @DATE
SET @TOT_BALANCE = @TOT_BALANCE + @DAYCLOSINGBALANCEVAL
END
SET @DATE = @DATE + 1
END
FETCH NEXT FROM CUR_ITEM INTO @ITEMCODE
END
CLOSE CUR_ITEM
DEALLOCATE CUR_ITEM
FETCH NEXT FROM CUR_ITEMGROUP INTO @ITEMGROUP
END
CLOSE CUR_ITEMGROUP
DEALLOCATE CUR_ITEMGROUP
FETCH NEXT FROM CUR_WAREHOUSE INTO @WAREHOUSE
END
CLOSE CUR_WAREHOUSE
DEALLOCATE CUR_WAREHOUSE
DECLARE @WH_TS NVARCHAR(20), @ITEMGROUP_TS NVARCHAR(20),@ITEMCODE_TS NVARCHAR(20), @DATE_TS DATETIME, @OB_TS NUMERIC(16,4), @OB_VAL_TS NUMERIC(16,4), @IN_QTY_TS NUMERIC(16,4), @IN_QTY_VAL_TS NUMERIC(16,4), @OUT_QTY_TS NUMERIC(16,4), @OUT_QTY_VAL_TS NUMERIC(16,4), @CB_QTY_TS NUMERIC(16,4), @CB_QTY_VAL_TS NUMERIC(16,4), @INV_REV_FLAG_TS INT
DECLARE @TEMP NUMERIC(16,4)
DECLARE @DATE_IN DATETIME
DECLARE @FIRST_REC INT
DECLARE @OB_QTY_IN NUMERIC(16,4), @OB_VAL_IN NUMERIC(16,4), @CL_QTY_IN NUMERIC(16,4), @CL_VAL_IN NUMERIC(16,4)
DECLARE @TOT_IN_QTY_IN NUMERIC(16,4), @TOT_IN_VAL_IN NUMERIC(16,4), @TOT_OUT_QTY_IN NUMERIC(16,4), @TOT_OUT_VAL_IN NUMERIC(16,4), @TOT_INV_REV_FALG INT
DECLARE @GTOT_IN_QTY_IN NUMERIC(16,4), @GTOT_IN_VAL_IN NUMERIC(16,4), @GTOT_OUT_QTY_IN NUMERIC(16,4), @GTOT_OUT_VAL_IN NUMERIC(16,4)
DECLARE @GRPNAME NVARCHAR(100), @ITMDESC NVARCHAR(100)
DECLARE @GRAND_TOT_IN NUMERIC(16,4), @GRAND_TOT_OUT NUMERIC(16,4)
SET @FIRST_REC = 0
SET @GRAND_TOT_IN = 0
SET @GRAND_TOT_OUT = 0
--Warehouse Cursor
DECLARE CUR_WAREHOUSE CURSOR FOR SELECT WHSCODE FROM OWHS WHERE ((WHSCODE >= @F_WAREHOUSE AND WHSCODE <= @T_WAREHOUSE) OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = '')) ORDER BY WHSCODE
OPEN CUR_WAREHOUSE
FETCH NEXT FROM CUR_WAREHOUSE INTO @WAREHOUSE
WHILE(@@FETCH_STATUS = 0)
BEGIN
--Item Group Cursor
DECLARE CUR_ITEMGROUP CURSOR FOR SELECT ITMSGRPCOD FROM OITB
WHERE ((ITMSGRPCOD = @F_ITEMGROUP AND ITMSGRPCOD <= @T_ITEMGROUP)
OR (@F_ITEMGROUP = ''AND @T_ITEMGROUP = ''))
ORDER BY ITMSGRPCOD
OPEN CUR_ITEMGROUP
FETCH NEXT FROM CUR_ITEMGROUP INTO @ITEMGROUP
WHILE(@@FETCH_STATUS = 0)
BEGIN
SET @GTOT_IN_QTY_IN = 0
SET @GTOT_IN_VAL_IN = 0
SET @GTOT_OUT_QTY_IN = 0
SET @GTOT_OUT_VAL_IN = 0
--Item Cursor
DECLARE CUR_ITEM CURSOR FOR SELECT DISTINCT T6.ITEMCODE FROM OINM T6 WHERE ((T6.ITEMCODE IN (SELECT T0.[ItemCode] FROM OITM T0 INNER JOIN OITB T1 ON T0.ItmsGrpCod = T1.ItmsGrpCod WHERE T1.ITMSGRPCOD = @ITEMGROUP)) AND (T6.WAREHOUSE = @WAREHOUSE)) ORDER BY T6.ITEMCODE -- AND ((T6.DOCDATE >= @F_DATE AND T6.DOCDATE <= @T_DATE) OR (@F_DATE = '' AND @T_DATE = '') )) ORDER BY T6.ITEMCODE
OPEN CUR_ITEM
FETCH NEXT FROM CUR_ITEM INTO @ITEMCODE
WHILE(@@FETCH_STATUS = 0)
BEGIN
SET @TOT_IN_QTY_IN = 0
SET @TOT_IN_VAL_IN = 0
SET @TOT_OUT_QTY_IN = 0
SET @TOT_OUT_VAL_IN = 0
SET @TOT_INV_REV_FALG = 0
SET @FIRST_REC = 0
DECLARE CUR_DATE_IN CURSOR FOR SELECT DISTINCT DATE FROM @TAB_DAY_STOCK WHERE (WH = @WAREHOUSE) AND (ITEMGROUP = @ITEMGROUP)AND (ITEMCODE = @ITEMCODE) AND ((DATE >= @F_DATE AND DATE <= @T_DATE) OR (@F_DATE = '' AND @T_DATE = '') )
OPEN CUR_DATE_IN
FETCH NEXT FROM CUR_DATE_IN INTO @DATE_IN
WHILE(@@FETCH_STATUS = 0)
BEGIN
DECLARE CUR_TRANS CURSOR FOR SELECT * FROM @TAB_DAY_STOCK WHERE ((WH = @WAREHOUSE) AND (ITEMGROUP = @ITEMGROUP) AND (DATE = @DATE_IN) AND (ITEMCODE = @ITEMCODE))
OPEN CUR_TRANS
FETCH NEXT FROM CUR_TRANS INTO @WH_TS,@packcartons, @ITEMGROUP_TS,@ITEMCODE_TS, @DATE_TS, @OB_TS, @OB_VAL_TS, @IN_QTY_TS, @IN_QTY_VAL_TS, @OUT_QTY_TS, @OUT_QTY_VAL_TS, @CB_QTY_TS, @CB_QTY_VAL_TS, @INV_REV_FLAG_TS,@itmprice
WHILE (@@FETCH_STATUS = 0)
BEGIN
SET @FIRST_REC = @FIRST_REC + 1
IF @FIRST_REC = 1
BEGIN
SET @OB_QTY_IN = @OB_TS
SET @OB_VAL_IN = @OB_VAL_TS
END
SET @TOT_IN_QTY_IN = @TOT_IN_QTY_IN + @IN_QTY_TS
SET @TOT_IN_VAL_IN = @TOT_IN_VAL_IN + @IN_QTY_VAL_TS
SET @TOT_OUT_QTY_IN = @TOT_OUT_QTY_IN + @OUT_QTY_TS
SET @TOT_OUT_VAL_IN = @TOT_OUT_VAL_IN + @OUT_QTY_VAL_TS
SET @GTOT_IN_QTY_IN = @GTOT_IN_QTY_IN + @IN_QTY_TS
SET @GTOT_IN_VAL_IN = @GTOT_IN_VAL_IN + @IN_QTY_VAL_TS
SET @GTOT_OUT_QTY_IN = @GTOT_OUT_QTY_IN + @OUT_QTY_TS
SET @GTOT_OUT_VAL_IN = @GTOT_OUT_VAL_IN + @OUT_QTY_VAL_TS
SET @CL_QTY_IN = @CB_QTY_TS
SET @CL_VAL_IN = @CB_QTY_VAL_TS
SET @TOT_INV_REV_FALG = @TOT_INV_REV_FALG + @INV_REV_FLAG_TS
FETCH NEXT FROM CUR_TRANS INTO @WH_TS,@packcartons, @ITEMGROUP_TS,@ITEMCODE_TS, @DATE_TS, @OB_TS, @OB_VAL_TS, @IN_QTY_TS, @IN_QTY_VAL_TS, @OUT_QTY_TS, @OUT_QTY_VAL_TS, @CB_QTY_TS, @CB_QTY_VAL_TS, @INV_REV_FLAG_TS,@itmprice
END
CLOSE CUR_TRANS
DEALLOCATE CUR_TRANS
FETCH NEXT FROM CUR_DATE_IN INTO @DATE_IN
END
CLOSE CUR_DATE_IN
DEALLOCATE CUR_DATE_IN
SET @GRPNAME = (SELECT ITMSGRPNAM FROM OITB WHERE ITMSGRPCOD = @ITEMGROUP)
SET @ITMDESC = (SELECT ITEMNAME FROM OITM WHERE ITEMCODE = @ITEMCODE)
declare cur_item_packunit cursor for select SALPACKUN from oitm where itemcode = @ITEMCODE
open cur_item_packunit
fetch next from cur_item_packunit into @packunit
while(@@FETCH_STATUS = 0)
begin
SET @packcartons = @CL_QTY_IN/@packunit
fetch next from cur_item_packunit into @packunit
end
close cur_item_packunit
deallocate cur_item_packunit
---- item price
declare cur_item_price cursor for select price from itm1 where itemcode = @ITEMCODE
open cur_item_price
fetch next from cur_item_price into @itmprice
set @itmprice=@itmprice
close cur_item_price
deallocate cur_item_price
-----------------
INSERT INTO @TAB_STOCK VALUES(@WH_TS,@GRPNAME, @ITEMCODE_TS, @ITMDESC, @OB_QTY_IN, @OB_VAL_IN, @TOT_IN_QTY_IN, @TOT_IN_VAL_IN, @GTOT_IN_QTY_IN, @GTOT_IN_VAL_IN, @TOT_OUT_QTY_IN, @TOT_OUT_VAL_IN, @GTOT_OUT_QTY_IN, @GTOT_OUT_VAL_IN, @CL_QTY_IN, @CL_VAL_IN,@packcartons,@itmprice,@ITEMGROUP,@DATE)
IF @TOT_IN_QTY_IN != 0 OR @TOT_OUT_QTY_IN != 0 OR @TOT_INV_REV_FALG > 0
BEGIN
SET @TOT_TRANS_BALANCE = @TOT_TRANS_BALANCE + @CL_VAL_IN
END
FETCH NEXT FROM CUR_ITEM INTO @ITEMCODE
END
CLOSE CUR_ITEM
DEALLOCATE CUR_ITEM
IF @GTOT_IN_VAL_IN != 0 OR @GTOT_OUT_VAL_IN != 0
BEGIN
SET @GRAND_TOT_IN = @GRAND_TOT_IN + @GTOT_IN_VAL_IN
SET @GRAND_TOT_OUT = @GRAND_TOT_OUT + @GTOT_OUT_VAL_IN
END
FETCH NEXT FROM CUR_ITEMGROUP INTO @ITEMGROUP
END
CLOSE CUR_ITEMGROUP
DEALLOCATE CUR_ITEMGROUP
FETCH NEXT FROM CUR_WAREHOUSE INTO @WAREHOUSE
END
CLOSE CUR_WAREHOUSE
DEALLOCATE CUR_WAREHOUSE
Return
END
February 5, 2013 at 12:04 am
I'm not suprised it took a couple of hours looking at the query you have loops with cursors which is going to drag performance down into the basement and beyond then on top of that the SQL function reads like a VB program function.
A couple of blatently obvious things apart from all the loops and Cursors, things that are going to kill this include this type of thing.
SET @ST_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE)
SET @EN_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE DESC)
Thats two queries with a sort operator, and they can be better expressed, on top of that this is run again in the next block, which is superflous as I cannot see where the @ST_DATE, and @EN_DATE changes in the interim.
SELECT @ST_DATE=MAX(DOCDATE), @EN_DATE=MIN(DOCDATE) FROM OINM
Evaluate the Top part on its own and then evaluate the second query I'll put money on the second part of code beating the top one.
In short this needs a total rewrite so that its less procedural and more Set based.
_________________________________________________________________________
SSC Guide to Posting and Best Practices
February 5, 2013 at 1:08 am
Jason-299789 (2/5/2013)
I'm not suprised it took a couple of hours looking at the query you have loops with cursors which is going to drag performance down into the basement and beyond then on top of that the SQL function reads like a VB program function.A couple of blatently obvious things apart from all the loops and Cursors, things that are going to kill this include this type of thing.
SET @ST_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE)
SET @EN_DATE = (SELECT TOP 1 DOCDATE FROM OINM ORDER BY DOCDATE DESC)
Thats two queries with a sort operator, and they can be better expressed, on top of that this is run again in the next block, which is superflous as I cannot see where the @ST_DATE, and @EN_DATE changes in the interim.
SELECT @ST_DATE=MAX(DOCDATE), @EN_DATE=MIN(DOCDATE) FROM OINM
Evaluate the Top part on its own and then evaluate the second query I'll put money on the second part of code beating the top one.
In short this needs a total rewrite so that its less procedural and more Set based.
yes, I thought I would be seeing a single cursor,after formatting the code ;but, it was cusror within a cursor , within a cursor and so on..:w00t:
You should restructure the query ..
Use case statements to get the results and calculation ; Like inner most cursor can be do away with case statements in the select ..
use temptables to store data ; does it have to be a function ? I think, stored procedure should be better in this case .
And as jason suggested , @st_date and @en_date would be same wherever you set it ; in the inner most cursor , adding the same , would be executed each time for each item read by cursor..
For restructuring , If you need a result set to work on with then have temptable, try to work in batches , avoid cursors, if possible..
And, lastly , I hope DOCDATE is not null column , otherwise , it would be a mess to carry on
as @date = @date+1 ; that is why taking max(date) or min(date) is better ...
~ demonfox
___________________________________________________________________
Wondering what I would do next , when I am done with this one :ermm:
February 5, 2013 at 1:30 am
No less than four levels of nested queries! Good Lord, no wonder it takes so long. Examining the code, none of it, even the few queries, look as if they've been written by an experienced SQL developer and there are mistakes - compare these two cursor definitions:
-- CUR_WAREHOUSE
SELECT WHSCODE -- @WAREHOUSE
FROM OWHS
WHERE ((WHSCODE >= @F_WAREHOUSE AND WHSCODE <= @T_WAREHOUSE)
OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = ''))
ORDER BY WHSCODE
-- CUR_ITEMGROUP
SELECT ITMSGRPCOD -- @ITEMGROUP
FROM OITB
WHERE ((ITMSGRPCOD = @F_ITEMGROUP AND ITMSGRPCOD <= @T_ITEMGROUP)
OR (@F_ITEMGROUP = ''AND @T_ITEMGROUP = ''))
ORDER BY ITMSGRPCOD
The second definition is supposed to have a range in the WHERE clause.
I'd suggest that if this function generates correct results, then it's by accident, not by design. However, it shouldn't be too tough to convert it to a proper set-based function. The four nested cursors can be combined something like this:
SELECT w.WHSCODE, T6.ITEMCODE, T0.ItmsGrpCod,
(CONVERT(NUMERIC(16,4), T6.INQTY)), -- @INQTY
(CONVERT(NUMERIC(16,4), T6.OUTQTY)), -- @OUTQTY
(CONVERT(NUMERIC(16,4), T6.TRANSVALUE)), -- @CALCPRICE
T6.TRANSTYPE -- @TRANSTYPE
FROM OINM T6
INNER JOIN OWHS w
ON w.WHSCODE = T6.WAREHOUSE
INNER JOIN OITM T0
ON T0.[ItemCode] = T6.ITEMCODE
AND T0.ItmsGrpCod = @ITEMGROUP
WHERE (w.WHSCODE >= @F_WAREHOUSE AND w.WHSCODE <= @T_WAREHOUSE)
OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = '')
The arithmetic appears to be fairly simple, I can't see any reason why it can't be done in the output.
This is just the first phase. The output of this first chunk of the function is used to drive a second bank of nested cursors which perform more calculations.
This looks like a VB programmer's first attempt at TSQL coding. Get a professional in to write it properly.
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
February 5, 2013 at 1:39 am
and chances this shouldn't be a SQLFunction at all, but a regular stored procedure.
Keep in mind, if a function doesn't perform well, it will drag down the query using it ... and ALL queries using it !
Johan
Learn to play, play to learn !
Dont drive faster than your guardian angel can fly ...
but keeping both feet on the ground wont get you anywhere :w00t:
- How to post Performance Problems
- How to post data/code to get the best help[/url]
- How to prevent a sore throat after hours of presenting ppt
press F1 for solution, press shift+F1 for urgent solution 😀
Need a bit of Powershell? How about this
Who am I ? Sometimes this is me but most of the time this is me
February 5, 2013 at 1:44 am
ChrisM@Work (2/5/2013)
No less than four levels of nested queries! Good Lord, no wonder it takes so long. Examining the code, none of it, even the few queries, look as if they've been written by an experienced SQL developer and there are mistakes - compare these two cursor definitions:
-- CUR_WAREHOUSESELECT WHSCODE -- @WAREHOUSE
FROM OWHS
WHERE ((WHSCODE >= @F_WAREHOUSE AND WHSCODE <= @T_WAREHOUSE)
OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = ''))
ORDER BY WHSCODE
-- CUR_ITEMGROUP
SELECT ITMSGRPCOD -- @ITEMGROUP
FROM OITB
WHERE ((ITMSGRPCOD = @F_ITEMGROUP AND ITMSGRPCOD <= @T_ITEMGROUP)
OR (@F_ITEMGROUP = ''AND @T_ITEMGROUP = ''))
ORDER BY ITMSGRPCOD
The second definition is supposed to have a range in the WHERE clause.
I'd suggest that if this function generates correct results, then it's by accident, not by design. However, it shouldn't be too tough to convert it to a proper set-based function. The four nested cursors can be combined something like this:
SELECT w.WHSCODE, T6.ITEMCODE, T0.ItmsGrpCod,(CONVERT(NUMERIC(16,4), T6.INQTY)), -- @INQTY
(CONVERT(NUMERIC(16,4), T6.OUTQTY)), -- @OUTQTY
(CONVERT(NUMERIC(16,4), T6.TRANSVALUE)), -- @CALCPRICE
T6.TRANSTYPE -- @TRANSTYPE
FROM OINM T6
INNER JOIN OWHS w
ON w.WHSCODE = T6.WAREHOUSE
INNER JOIN OITM T0
ON T0.[ItemCode] = T6.ITEMCODE
AND T0.ItmsGrpCod = @ITEMGROUP
WHERE (w.WHSCODE >= @F_WAREHOUSE AND w.WHSCODE <= @T_WAREHOUSE)
OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = '')
The arithmetic appears to be fairly simple, I can't see any reason why it can't be done in the output.
This is just the first phase. The output of this first chunk of the function is used to drive a second bank of nested cursors which perform more calculations.
This looks like a VB programmer's first attempt at TSQL coding. Get a professional in to write it properly.
+1, especially about this looking like a procedural programmers attempt at SQL, wheres JC when you really need him 😛
_________________________________________________________________________
SSC Guide to Posting and Best Practices
February 5, 2013 at 1:52 am
Just a generic observation...
If this function is taking two hours to run--even with all of the cursors and subcursors--then you are probably accessing a LOT of data. I don't think a function is even appropriate. Is this being called by another procedure or query? If so the issues raised are multiplied by how many times the function itself is being called.
You should write a procedure instead. Take the loops and concentrate on writing queries that don't use loops/cursors. Perhaps you break the queries into smaller pieces and insert the results into temp tables just to get the data someplace where you can query it efficiently. If you have a truly huge amount of data it would be even better to insert it into an actual table to take a load off tempdb. Once broken down into smaller pieces it will be a LOT easier to do your performance tuning as well.
Also, it may help performance if all of your DDL (DECLARE statements, CREATE table, etc) are defined before you do any DML (the procedural "stuff"). Mixing up DDL and DML, especially in such a huge amount of code, can cause recompilation issues.
I don't know what kind of results the pseudo-queries below will give you, but you should strive for something like these rather than so many cursors. I've used cursors and still do occasionally but I rarely nest cursors. Set based queries will always beat loops of any kind when it comes to performance. Sometimes performance isn't an issue due to small scale or infrequent runs etc. But clearly this function has evolved into a monster.
--try joining instead of nesting loops
SELECT
OWHS.WHSCODE
,OITB.ITMSGRPCOD
FROM
OWHS
INNER JOIN
OITB
ON OWHS.WHSCODE = OITB.WHSCODE
WHERE
1=1
AND
((OWHS.WHSCODE >= @F_WAREHOUSE
OWHS.AND WHSCODE <= @T_WAREHOUSE)
OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = ''))
AND
((OITB.ITMSGRPCOD = @F_ITEMGROUP
AND OITB.ITMSGRPCOD <= @T_ITEMGROUP)
OR (@F_ITEMGROUP = '' AND @T_ITEMGROUP = ''))
ORDER BY
OWHS.WHSCODE
,OITB.ITMSGRPCOD
--OR
SELECT
WHSCODE
,ITMSGRPCOD
FROM
(
SELECT
OWHS.WHSCODE
FROM
OWHS
WHERE
((OWHS.WHSCODE >= @F_WAREHOUSE
OWHS.AND WHSCODE <= @T_WAREHOUSE)
OR (@F_WAREHOUSE = '' AND @T_WAREHOUSE = ''))
) AS OWHS
INNER JOIN
(
SELECT
OITB.ITMSGRPCOD
FROM
OITB
WHERE
((OITB.ITMSGRPCOD = @F_ITEMGROUP
AND OITB.ITMSGRPCOD <= @T_ITEMGROUP)
OR (@F_ITEMGROUP = '' AND @T_ITEMGROUP = ''))
) AS OITB
ON OWHS.WHSCODE = OITB.WHSCODE
ORDER BY
OWHS.WHSCODE
,OITB.ITMSGRPCOD
February 5, 2013 at 2:45 am
Steven Willis (2/5/2013)
Just a generic observation...If this function is taking two hours to run--even with all of the cursors and subcursors--then you are probably accessing a LOT of data. I don't think a function is even appropriate. Is this being called by another procedure or query? If so the issues raised are multiplied by how many times the function itself is being called.
You should write a procedure instead. ..
Not necessarily. A properly written iTVF inlines like a view. This function is just a 4-table query with a few sums in it. I can't see any reason why it shouldn't be written as an iTVF regardless of how many rows the base tables contain.
For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden
February 5, 2013 at 3:47 am
Just remember that the user defined muti-statement table valued function works off the construct of the table variable. the table variables one defining characteristic is it's lack of statistics. So, any query run against a multi-statement table valued function (or a table variable) that does any kind of filtering or joining or any operation that requires statistics, then the optimizer, having no statistics to work with, assumes one (1) row. This is fine if you're working with 1-10 rows, but as soon as those values grow, this method becomes an ENORMOUS performance bottleneck.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
February 5, 2013 at 11:23 am
ChrisM@Work (2/5/2013)
Steven Willis (2/5/2013)
Just a generic observation...If this function is taking two hours to run--even with all of the cursors and subcursors--then you are probably accessing a LOT of data. I don't think a function is even appropriate. Is this being called by another procedure or query? If so the issues raised are multiplied by how many times the function itself is being called.
You should write a procedure instead. ..
Not necessarily. A properly written iTVF inlines like a view. This function is just a 4-table query with a few sums in it. I can't see any reason why it shouldn't be written as an iTVF regardless of how many rows the base tables contain.
Can't argue with that. Frankly, I didn't even look at the code in detail because it was late and 600 lines of cursors had me overwhelmed. :crazy:
If all of that code boils down to 4-tables then perhaps an iTVF would work fine--though I'm astonished really that such a case could wind up looking like the code that was posted. It would also validate the main point of my post in which I tried to show the OP a couple of pseudo-code examples of using JOINS rather than cursors.
February 5, 2013 at 5:10 pm
sanjaytiwari31 (2/4/2013)
Can anybody tell me how to optimize this function or where is the problem.
Yes I can but you're not going to like it. The code cannot be optimized no matter how much you play with it because of the nested cursors. It would be a far better thing sack the code entirely and rewrite it from the ground up without the cursors.
Because of the length of the code, I've not done a serious deep dive on it but I agree with what has already been said... it looks like it could be resolved by a single relatively simple query across 4 tables. If no one in your shop can handle the conversion, your shop should get some hired help. 😛
--Jeff Moden
Change is inevitable... Change for the better is not.
February 6, 2013 at 3:23 am
Try to create a primary key/unique key on your table variable, and it is always better to go as a stored proc using temp tables and while loop instead of cursors and try building a clustered index on the temp table which will definitely improve your performance.
February 6, 2013 at 4:05 am
k.srikalyan (2/6/2013)
Try to create a primary key/unique key on your table variable, and it is always better to go as a stored proc using temp tables and while loop instead of cursors and try building a clustered index on the temp table which will definitely improve your performance.
A while loop is a cursor. That's not a replacement for one.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
February 6, 2013 at 5:31 am
Not really looking in detail but
I think it should be a store procedure.
And I quite sure we can eliminated the cursor using temp table and update and set variables in a single query.
February 6, 2013 at 8:24 am
k.srikalyan (2/6/2013)
Try to create a primary key/unique key on your table variable, and it is always better to go as a stored proc using temp tables and while loop instead of cursors and try building a clustered index on the temp table which will definitely improve your performance.
While loops are not all that better than cursors, it is still RBAR.
Viewing 15 posts - 1 through 15 (of 18 total)
You must be logged in to reply to this topic. Login to reply