Click here to monitor SSC
SQLServerCentral is supported by Red Gate Software Ltd.
 
Log in  ::  Register  ::  Not logged in
 
 
 
        
Home       Members    Calendar    Who's On


Add to briefcase 12»»

SQL Function takes more than 2 hours to return a table.. Expand / Collapse
Author
Message
Posted Monday, February 4, 2013 10:56 PM
Forum Newbie

Forum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum NewbieForum Newbie

Group: General Forum Members
Last Login: Tuesday, February 5, 2013 12:45 AM
Points: 1, Visits: 1
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:

Alter FUNCTION [dbo].[TEST_STOCK_SUMMARY_NEW](@F_WAREHOUSE NVARCHAR(20), @T_WAREHOUSE NVARCHAR(20), @F_ITEMGROUP NVARCHAR(20), @T_ITEMGROUP NVARCHAR(20), @F_DATE DATETIME, @T_DATE DATETIME)
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



Post #1415576
Posted Tuesday, February 5, 2013 12:04 AM
SSC Eights!

SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!

Group: General Forum Members
Last Login: Today @ 9:31 AM
Points: 922, Visits: 2,524
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
Post #1415589
Posted Tuesday, February 5, 2013 1:08 AM
Ten Centuries

Ten CenturiesTen CenturiesTen CenturiesTen CenturiesTen CenturiesTen CenturiesTen CenturiesTen Centuries

Group: General Forum Members
Last Login: Thursday, July 31, 2014 7:15 PM
Points: 1,129, Visits: 1,163
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..

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
Post #1415625
Posted Tuesday, February 5, 2013 1:30 AM


SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: General Forum Members
Last Login: Today @ 9:43 AM
Points: 6,869, Visits: 14,178
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.


“Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail Shaw

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
Exploring Recursive CTEs by Example Dwain Camps
Post #1415638
Posted Tuesday, February 5, 2013 1:39 AM


SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: General Forum Members
Last Login: Today @ 12:21 AM
Points: 6,743, Visits: 8,517
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


Don't drive faster than your guardian angel can fly ...
but keeping both feet on the ground won't get you anywhere

- How to post Performance Problems
- How to post data/code to get the best help


- 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
Post #1415642
Posted Tuesday, February 5, 2013 1:44 AM
SSC Eights!

SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!SSC Eights!

Group: General Forum Members
Last Login: Today @ 9:31 AM
Points: 922, Visits: 2,524
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_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.


+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
Post #1415645
Posted Tuesday, February 5, 2013 1:52 AM
SSC-Addicted

SSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-Addicted

Group: General Forum Members
Last Login: Sunday, September 29, 2013 1:24 AM
Points: 429, Visits: 1,721
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


Post #1415646
Posted Tuesday, February 5, 2013 2:45 AM


SSCertifiable

SSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiableSSCertifiable

Group: General Forum Members
Last Login: Today @ 9:43 AM
Points: 6,869, Visits: 14,178
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.


“Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail Shaw

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
Exploring Recursive CTEs by Example Dwain Camps
Post #1415668
Posted Tuesday, February 5, 2013 3:47 AM


SSChampion

SSChampionSSChampionSSChampionSSChampionSSChampionSSChampionSSChampionSSChampionSSChampionSSChampion

Group: General Forum Members
Last Login: Today @ 11:01 AM
Points: 14,029, Visits: 28,404
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
The Scary DBA
Author of: SQL Server Query Performance Tuning
SQL Server 2012 Query Performance Tuning
SQL Server 2008 Query Performance Tuning Distilled
and
SQL Server Execution Plans

Product Evangelist for Red Gate Software
Post #1415693
Posted Tuesday, February 5, 2013 11:23 AM
SSC-Addicted

SSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-AddictedSSC-Addicted

Group: General Forum Members
Last Login: Sunday, September 29, 2013 1:24 AM
Points: 429, Visits: 1,721
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.

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.

 
Post #1416014
« Prev Topic | Next Topic »

Add to briefcase 12»»

Permissions Expand / Collapse