Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

 All Forums
 SQL Server 2000 Forums
 SQL Server Development (2000)
 Getting rid of cursors and optimizing performance

Author  Topic 

jzurbo77
Starting Member

21 Posts

Posted - 2008-06-17 : 17:00:17
Hello

I inherited a large chunk of SQL code on MSSQL2000 with three cursors working daily on a 28GB table. It works almost 4 hours (at night). From what I am reading on forums like this one I conclude that the code can (and should) be optimized, including replacing cursors and sometimes it reduces run time tenfold or more. If someone would be willing to take a look at the code and suggest steps to optimize it I would be happy to post it...

any assistance would be appreciated

any takers?

Thanks
John

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2008-06-17 : 17:02:37
We certainly can't get started without the code. Make sure to post it using code tags so that your code formatting is retained.

Also you might not get an answer if you don't provide a detailed description of what it is doing, the DDL for all tables involved, sample data for each of those tables using INSERT INTO statements, and the expected output using that sample data.

Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog
Go to Top of Page

jzurbo77
Starting Member

21 Posts

Posted - 2008-06-17 : 17:14:48
Thanks for responding so quickly!!

Below is the code. I will post whatever you request, not sure what exactly you need...
Let's try one step at a time - if you can suggest something by looking at the code first...



set ANSI_NULLS ON
set QUOTED_IDENTIFIER ON
go
/*
Module Name: spzCoaMain
Author:
Purpose: Retrieve data needed for the COA reports
-------------
Maintenance Log
-------------
11/13/06 v01 New.
05/12/08 Re-written for Somerset COA
06/17/08 v16
06/17/08 v17 AND (cu.Customer_General_1 IS NOT NULL OR @txtParam_CoaCust = 'All' OR @txtParam_CoaCust IS NULL)
===========================

CREATE TABLE #statResults (VAR_ID int,ORDER_NUMBER varchar(50) , SHIPMENT_DATE datetime , U_Entry real,
U_Reject real,U_Warning real,U_User real,Target real,L_Entry real,L_Reject real,L_Warning real,L_User real)

CREATE TABLE #testResults (VAR_ID int, EVENT_ID int, RES varchar(50),ORDER_NUMBER varchar(50),SHIPMENT_DATE datetime )

exec dbo.spzCoaMain '2007-09-15', '2007-09-18',-1 , 'All', 'All', 'All', 'COA', NULL, 'DEBUG'
-- DateFrom DateTo CustID Cust Plant Roll COA Rep Run Mode
-- Ord Ord Nari Cust? Type
-- Numb Numb ID
select * from #statResults
select * from #testResults
drop table #testResults
drop table #statResults

=======================================
-------------
*/
ALTER PROCEDURE [dbo].[spzCoaMain]
@dteParam_DateFrom DATETIME,
@dteParam_DateTo DATETIME,
@intParam_Customer_Id INT,
@txtParam_Customer_Order_Number VARCHAR(50),
@txtParam_Plant_Order_Number VARCHAR(50),
@txtParam_Roll_Nari_Id VARCHAR(50),
@txtParam_COACust CHAR(3),
@txtParam_ReportType VARCHAR(3), --COA or PRO
@txtParam_RunMode VARCHAR(5) --Load with DEBUG to view PRINT statements
AS
SET CONCAT_NULL_YIELDS_NULL OFF
SET NOCOUNT ON

--exec dbo.spzGetDayOrders '2007-10-03'

DECLARE @orderNumber varchar(50)
, @shipmentDate DATETIME
, @Prim_Event_Number varchar(50)
, @Srce_Event_Number varchar(50)
DECLARE @ts datetime
, @ts_temp datetime
, @event_id int
, @var_id int
, @pu_id int
, @source_event_id int
DECLARE @product_run int --datetime
, @product_run_original int --datetime
, @viewed_neighbours int

DECLARE @work_ts DATETIME, @direction varchar(50), @cr_delta int, @cr_ts datetime,
@forward1_work_ts DATETIME, @backward1_work_ts DATETIME

if object_id('tempdb..#headers') IS NOT NULL
DROP TABLE #headers

CREATE TABLE #headers (
U_Entry varchar(25)
, U_Reject varchar(25)
, U_Warning varchar(25)
, U_User varchar(25)
, Target varchar(25)
, L_Entry varchar(25)
, L_Reject varchar(25)
, L_Warning varchar(25)
, L_User varchar(25))

if object_id('tempdb..#parentRoll') IS NOT NULL
DROP TABLE #parentRoll

CREATE TABLE #parentRoll (
Primary_Event_num varchar(25)
, event_id int
, pu_id int
, [timestamp] datetime
, Original_product varchar(25))

--CREATE TABLE #testResults (VAR_ID int, EVENT_ID int, RES varchar(50))
--CREATE TABLE #statResults (VAR_ID int, STAT_NAME varcgar(50), RES varchar(50))

DECLARE @tstValue VARCHAR(25)
DECLARE @tsForHeader DATETIME
, @prod_id INT
, @step INT
, @neighbour_step CHAR(1) -- a/b/c/d 8612
, @forward_flag CHAR(3) -- YES/NO 8612
, @backward_flag CHAR(3) -- YES/NO 8612

-- FETCH ALL ORDERS

DECLARE allOrders CURSOR READ_ONLY FOR

select distinct co.plant_order_number
, s.shipment_date
from gbdb.dbo.Customer_Orders co
JOIN gbdb.dbo.Customer cu on co.customer_id = cu.customer_id
join gbdb.dbo.Event_Details ed on co.Order_Id = ed.Order_Id
JOIN gbdb.dbo.Shipment_Line_Items sli ON sli.shipment_item_id = ed.shipment_item_id
JOIN gbdb.dbo.Shipment s ON s.shipment_id = sli.shipment_id
--select plant_order_number from gbdb.dbo.Customer_Orders
WHERE s.Is_Active = 1 --Get Active (not canceled) Orders
AND sli.Is_Active = 1
AND (cu.Customer_Id = @intParam_Customer_Id OR @intParam_Customer_Id = -1)
AND (co.Customer_Order_Number = @txtParam_Customer_Order_Number OR @txtParam_Customer_Order_Number = 'All')
AND (co.Plant_Order_Number = @txtParam_Plant_Order_Number OR @txtParam_Plant_Order_Number = 'All')
AND (cu.Customer_General_1 IS NOT NULL OR @txtParam_CoaCust = 'All' OR @txtParam_CoaCust IS NULL)
AND (s.shipment_date >= @dteParam_DateFrom OR @dteParam_DateFrom IS NULL)
AND (s.shipment_date < @dteParam_DateTo OR @dteParam_DateTo IS NULL)
AND (ed.Primary_Event_Num = @txtParam_Roll_Nari_Id or @txtParam_Roll_Nari_Id = 'All')

-- this query must be changed to return orders matching input cases, current query for test only!

OPEN allOrders
FETCH NEXT FROM allOrders INTO @orderNumber, @shipmentDate
SET NOCOUNT ON
WHILE @@FETCH_STATUS = 0
BEGIN
-- For each order
-- 1) get list of variables
if object_id('tempdb..#variables') IS NOT NULL
DROP TABLE #variables

create table #variables (var_id int)
insert into #variables (var_id)
exec spzGetOrderVarIds @orderNumber

-- 2) get list of rolls
if object_id('tempdb..#rolls') IS NOT NULL
DROP TABLE #rolls

create table #rolls (
Primary_Event_Num varchar(25)
, event_id int
, pu_id int
, [TimeStamp] datetime
, original_product int
, source_event_id int)

insert into #rolls
exec dbo.spzGetOrderRolls @orderNumber, @txtParam_Roll_Nari_Id
--


SET @tsForHeader = NULL
SET @prod_id = NULL
SELECT @tsForHeader = [TimeStamp], @prod_id = original_product FROM #rolls

IF(@tsForHeader IS NOT NULL AND @prod_id IS NOT NULL)
BEGIN
DECLARE allVars CURSOR READ_ONLY FOR
select var_id from #variables
OPEN allVars
FETCH NEXT FROM allVars INTO @var_id
SET NOCOUNT ON
WHILE @@FETCH_STATUS = 0
BEGIN
TRUNCATE TABLE #headers
INSERT INTO #headers
exec spzGetVarSpecs @tsForHeader, @var_id, @prod_id

INSERT INTO #statResults (VAR_ID,ORDER_NUMBER, SHIPMENT_DATE, U_Entry,U_Reject,U_Warning,U_User,Target,L_Entry,L_Reject,L_Warning,L_User)
SELECT @var_id, @orderNumber,@shipmentDate, U_Entry,U_Reject,U_Warning,U_User,Target,L_Entry,L_Reject,L_Warning,L_User FROM #headers

FETCH NEXT FROM allVars INTO @var_id
END
CLOSE allVars
DEALLOCATE allVars
END

DECLARE allRolls CURSOR READ_ONLY FOR
select DISTINCT [TimeStamp], event_id, pu_id, var_id, source_event_id from #rolls, #variables
OPEN allRolls

FETCH NEXT FROM allRolls INTO @ts, @event_id, @pu_id, @var_id, @source_event_id
SET NOCOUNT ON
WHILE @@FETCH_STATUS = 0
BEGIN
SET @step = 1
SET @viewed_neighbours = 0
IF @txtParam_RunMode = 'DEBUG'
PRINT '========= ROLL: '+CAST(@event_id as VARCHAR(25))
-- 4a) TEST ROLL
SET @source_event_id = @event_id
if object_id('tempdb..#tst') IS NOT NULL
DROP TABLE #tst
CREATE TABLE #tst (RES varchar(25), DT datetime)


label4a:


IF @txtParam_RunMode = 'DEBUG'
BEGIN
SELECT @Prim_Event_Number = Event_Num from gbdb.dbo.Events where Event_Id = @source_event_id
PRINT 'Start testing ROLL: '+CAST(@source_event_id as VARCHAR(25))+' '+CAST(@Prim_Event_Number as VARCHAR(25))
PRINT 'Params: '+CAST(@ts as VARCHAR(25))+','+CAST(@source_event_id as VARCHAR(25))+','+CAST(@pu_id as VARCHAR(25))+','+CAST(@var_id as VARCHAR(25))
END
INSERT INTO #tst
exec spzGetTestResult @ts, @source_event_id, @pu_id, @var_id
IF(@@ROWCOUNT > 0)
BEGIN
IF @txtParam_RunMode = 'DEBUG'
PRINT 'Success!'
SELECT @tstValue = RES FROM #tst
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, @tstValue, @orderNumber, @shipmentDate)
END
ELSE
BEGIN
-- try parent roll
IF @txtParam_RunMode = 'DEBUG'
PRINT 'Test not found!'
IF @txtParam_RunMode = 'DEBUG'
BEGIN
print 'Try to search parent for roll: '+CAST(@source_event_id as VARCHAR(20))+' '+CAST(@Prim_Event_Number as VARCHAR(25))
END
TRUNCATE TABLE #parentRoll
INSERT INTO #parentRoll
exec spzGetParentRoll @source_event_id
IF(@@ROWCOUNT > 0)
BEGIN
SET @step = 2
SELECT @ts = [timestamp], @source_event_id = event_id, @pu_id = pu_id from #parentRoll
if object_id('tempdb..#prRun1') IS NOT NULL
DROP TABLE #prRun1
CREATE TABLE #prRun1 (Result varchar(25), Result_On DateTime)
INSERT INTO #prRun1
exec spzGetProductRun @ts, @source_event_id, @pu_id
--exec spzGetProductRun '2007-09-17 17:00:00.000', 6389207, 14
IF @txtParam_RunMode = 'DEBUG'
print '*** @ts='+CAST(@ts as VARCHAR(20))+' @src_ev_id='+CAST(@source_event_id as VARCHAR(20))+' @pu_id='+CAST(@pu_id as VARCHAR(20))
SELECT @product_run_original = Result from #prRun1 --_on from #prRun1
DROP TABLE #prRun1
IF @txtParam_RunMode = 'DEBUG'
print 'Parent found!'+CAST(@source_event_id as VARCHAR(20))
GOTO label4a
END
ELSE
BEGIN
-- parent not found -- for first step - we must have parent!
IF @txtParam_RunMode = 'DEBUG'
print 'Parent NOT found!'
IF(@step > 1)
BEGIN
-- search first forward neighbour
set @direction = 'forward1'
neighbour:

if object_id('tempdb..#clReel') IS NOT NULL
DROP TABLE #clReel
CREATE TABLE #clReel (
EVENT_ID INT
, [TimeStamp] DateTime
, dd INT)
if(@direction = 'forward1')
begin
set @cr_delta = 300
set @cr_ts = @ts -- default for forward1
end
if(@direction = 'backward1')
begin
set @cr_delta = -300
set @cr_ts = @ts
end
if(@direction = 'forward2')
begin
set @cr_delta = 300
set @cr_ts = @forward1_work_ts -- from previous step
end
if(@direction = 'backward2')
begin
set @cr_delta = -300
set @cr_ts = @backward1_work_ts -- from previous step
end


IF @txtParam_RunMode = 'DEBUG'
BEGIN
print 'Current direction: '+@direction
print 'Parametrs of call: @ts='+CAST(@cr_ts as VARCHAR(20))+', @pu_id='+CAST(@pu_id as VARCHAR(25))+' @prod_id='+CAST(@prod_id as VARCHAR(50))+' @cr_delta='+CAST(@cr_delta as VARCHAR(50))
END
TRUNCATE TABLE #clReel
INSERT INTO #clReel
exec spzGetClosestReel @cr_ts, @pu_id, @prod_id, @cr_delta -- @var_id

IF(@@ROWCOUNT > 0)

BEGIN
SET @viewed_neighbours = @viewed_neighbours + 1
SELECT @source_event_id = EVENT_ID, @work_ts = [TimeStamp] FROM #clReel
if(@direction = 'forward1')
SET @forward1_work_ts = @work_ts
if(@direction = 'backward1')
SET @backward1_work_ts = @work_ts

IF @txtParam_RunMode = 'DEBUG'
print 'Found neighbour roll: '+CAST(@source_event_id as VARCHAR(20))+' '+CAST(@Prim_Event_Number as VARCHAR(25))+' TS: '+CAST(@work_ts as VARCHAR(50))
if object_id('tempdb..#prRun2') IS NOT NULL
DROP TABLE #prRun2
CREATE TABLE #prRun2 (Result varchar(25), Result_On DateTime)
INSERT INTO #prRun2
exec spzGetProductRun @work_ts, @source_event_id, @pu_id

SELECT @product_run = Result, @ts_temp = Result_on from #prRun2 --_on from #prRun2

if object_id('tempdb..#prRun2') IS NOT NULL
DROP TABLE #prRun2

if object_id('tempdb..#tst_2') IS NOT NULL
DROP TABLE #tst_2
CREATE TABLE #tst_2 (RES varchar(25), DT datetime)
INSERT INTO #tst_2
exec spzGetTestResult @work_ts, @source_event_id, @pu_id, @var_id
IF(@@ROWCOUNT > 0)
BEGIN
IF(@product_run_original = @product_run and @viewed_neighbours < 5)
BEGIN
-- Equal!
IF @txtParam_RunMode = 'DEBUG'
print 'Neighbour reel has the same product_run and test exists!'

SELECT @tstValue = RES FROM #tst_2
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, @tstValue, @orderNumber, @shipmentDate)
goto next_roll
END
END
ELSE
BEGIN
-- product run not the same, or test not found
IF @txtParam_RunMode = 'DEBUG'
BEGIN
IF @viewed_neighbours >= 5
print 'Too many neighbours viewed ('+CAST(@viewed_neighbours as VARCHAR(1))+'). Not tested.'
ELSE
IF (@product_run_original = @product_run)
print 'Test for neigbour did not found'
else
print 'Product_run is different Original is: '+CAST(@product_run_original as VARCHAR(7))+', current is '+CAST(@product_run as VARCHAR(7))+'.'
END
if(@direction = 'backward2')
BEGIN
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, NULL, @orderNumber, @shipmentDate)
goto next_roll
END
if(@direction = 'forward2')
SET @direction = 'backward2'
if(@direction = 'backward1')
SET @direction = 'forward2'
if(@direction = 'forward1')
SET @direction = 'backward1'
-- if neigbours was not found for backward1 or forward1 - skip backward2 or forward2 too
if(@direction = 'forward2' and @forward1_work_ts IS NULL)
SET @direction = 'backward2'
if(@direction = 'backward2' and (@backward1_work_ts IS NULL or @backward1_work_ts = ''))
BEGIN
IF @txtParam_RunMode = 'DEBUG'
print 'Tested neigbour was not found!'
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, NULL, @orderNumber, @shipmentDate)
goto next_roll
END

IF @txtParam_RunMode = 'DEBUG'
print 'New direction is: '+@direction
goto neighbour
END
END
ELSE
BEGIN
-- UNTESTED
/*
IF @txtParam_RunMode = 'DEBUG'
print 'Neighbour reel not found'
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, NULL, @orderNumber, @shipmentDate)
*/

if(@direction = 'backward2')
BEGIN
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, NULL, @orderNumber, @shipmentDate)
goto next_roll
END
if(@direction = 'forward2')
SET @direction = 'backward2'
if(@direction = 'backward1')
SET @direction = 'forward2'
if(@direction = 'forward1')
SET @direction = 'backward1'
-- if neigbours was not found for backward1 or forward1 - skip backward2 or forward2 too
if(@direction = 'forward2' and @forward1_work_ts IS NULL)
SET @direction = 'backward2'
if(@direction = 'backward2' and (@backward1_work_ts IS NULL or @backward1_work_ts = ''))
BEGIN
IF @txtParam_RunMode = 'DEBUG'
print 'Tested neigbour was not found!'
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, NULL, @orderNumber, @shipmentDate)
goto next_roll
END

IF @txtParam_RunMode = 'DEBUG'
print 'New direction is: '+@direction


goto neighbour
END
DROP TABLE #clReel
END
ELSE
BEGIN
-- UNTESTED
IF @txtParam_RunMode = 'DEBUG'
print 'Start roll must have parent!'
INSERT INTO #testResults (VAR_ID, EVENT_ID, RES, ORDER_NUMBER, SHIPMENT_DATE)
VALUES (@var_id, @event_id, NULL, @orderNumber, @shipmentDate)
END
END
END
next_roll:
if object_id('tempdb..#tst') IS NOT NULL
DROP TABLE #tst
FETCH NEXT FROM allRolls INTO @ts, @event_id, @pu_id, @var_id, @source_event_id
END
CLOSE allRolls
DEALLOCATE allRolls

DROP TABLE #variables
DROP TABLE #rolls

FETCH NEXT FROM allOrders INTO @orderNumber, @shipmentDate
END
CLOSE allOrders
DEALLOCATE allOrders

--select * from #testResults
DROP TABLE #headers
DROP TABLE #parentRoll


Go to Top of Page

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2008-06-17 : 17:23:38
I'm not sure about others here, but I wouldn't be able to help you with this for free. The amount of code that you posted and its complexity would require quite a bit of time to even understand, let alone rewrite.

You should drastically simplify your problem if you'd like some free help.

Here's an example where I asked for help:
http://www.sqlteam.com/forums/topic.asp?TOPIC_ID=42516

My problem was a lot more complex, but I narrowed it down to just the necessities. Notice how I received quite a few solutions in a short amount of time. Also notice how I posted my question.



Tara Kizer
Microsoft MVP for Windows Server System - SQL Server
http://weblogs.sqlteam.com/tarad/

Subscribe to my blog
Go to Top of Page

jzurbo77
Starting Member

21 Posts

Posted - 2008-06-17 : 23:21:42
I did not expect anyone to re-write it for free, but your suggestion to narrow the problem down of course makes sense. I am just not sure I have enough knowledge to do that at this moment...

I would not mind to pay if the quote will be in my modest budget - could you give me a ballpark, please?

I will read more posts - seems like q good forum.

Thanks again!
Go to Top of Page

Lumbago
Norsk Yak Master

3271 Posts

Posted - 2008-06-18 : 08:33:42
First of all I'll have to say that optimizing a procedure like this will take forever for us here since we don't have access to your database and *something* tells me this will not be possible Second, even though cursors are regarded as "bad" there are times when you just have to process something on a row by row basis when the logic and the amount of work to be performed on each row is just too complex to do it in a set based manner (I might get objections on this one but I can live with that hehe). This doesn't mean that you shouldn't optimize and think setbased, but you need to gain a complete understanding of what the procedure does first and then start to rethink the entire process and see what parts can be done differently.

And before optimizing a monster procedure like this you might actually be better of starting with something smaller where the results are a little easier to achieve. If you start on this procedure and after a while find out that you have went down the wrong track you might have wasted alot of time...gaining knowledge first on something smaller might actually be a good idea.

And my final universal tip: divide and conquer!! Works every time...

- Lumbago
Go to Top of Page

jsmith8858
Dr. Cross Join

7423 Posts

Posted - 2008-06-18 : 08:50:29
There are about 10 places that execute other stored procedures within this code. No one cannot optimize code that makes calls to other code without seeing ALL of it and knowing the big picture and what it is all doing. The bottleneck isn't just using cursors in general, it is how the entire thing is written, including all of the other stored procs being called.

See:

http://weblogs.sqlteam.com/jeffs/archive/2008/06/05/sql-server-cursor-removal.aspx

for more on that.

Plus, as Lumbago said it is impossible to do much of anything without details about the schema, the purpose of this stored procedure, the logic it is using, the expected output, sample data, etc. The worst thing you can do is post a large batch of complicated code without any explanation at all and just ask "can someone optimize this for me?" ... The answer is usually "no", not without additional information and a large amount of effort on your part to explain the problem and isolate the key areas you need help with.

- Jeff
http://weblogs.sqlteam.com/JeffS
Go to Top of Page

Lumbago
Norsk Yak Master

3271 Posts

Posted - 2008-06-18 : 09:00:22
quote:
Originally posted by jsmith8858
...
http://weblogs.sqlteam.com/jeffs/archive/2008/06/05/sql-server-cursor-removal.aspx
Wow, this article is really good and illustrates my point *exactly*

- Lumbago
Go to Top of Page

jzurbo77
Starting Member

21 Posts

Posted - 2008-06-18 : 09:46:54
Thank you all. Your posts and Jeff's article on Cursor Busting is a big help. It pretty much confirmed what I was thinkng - T-SQL is probably a wrong platform for that application. It should be written as an .NET application with datasets (not datareaders) holding data in memory where random and forward/backward processing will be way more efficient. The code I posted was more proof of concept than a "move to production" thing. The stored procedures called from it are result of "divide and conquer" approach - I could not agree more, Lumbago. They are simple and short selects and work fast.
I know why this code works slow and how to speed it up, but I do not know how to do it in T-SQL where I do not have a lot of experience. What I need is to be able to keep resultset(s) in memory and address (readonly!) its cells in both directions using indexes, like in array. I know how to do it in .NET but so far am not sure it is even possible in T-SQL. That is probably much narrower question, is it?

Again, thanks for your right to the point responses!
Go to Top of Page
   

- Advertisement -