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.
Author |
Topic |
jzurbo77
Starting Member
21 Posts |
Posted - 2008-06-17 : 17:00:17
|
HelloI 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 appreciatedany takers?ThanksJohn |
|
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 KizerMicrosoft MVP for Windows Server System - SQL Serverhttp://weblogs.sqlteam.com/tarad/Subscribe to my blog |
 |
|
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 ONset QUOTED_IDENTIFIER ONgo/*Module Name: spzCoaMainAuthor: 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 IDselect * from #statResults select * from #testResultsdrop table #testResultsdrop 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 statementsASSET CONCAT_NULL_YIELDS_NULL OFFSET 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 intDECLARE @product_run int --datetime , @product_run_original int --datetime , @viewed_neighbours intDECLARE @work_ts DATETIME, @direction varchar(50), @cr_delta int, @cr_ts datetime,@forward1_work_ts DATETIME, @backward1_work_ts DATETIMEif object_id('tempdb..#headers') IS NOT NULL DROP TABLE #headersCREATE 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 #parentRollCREATE 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 ORDERSDECLARE allOrders CURSOR READ_ONLY FORselect 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 allOrdersFETCH NEXT FROM allOrders INTO @orderNumber, @shipmentDateSET NOCOUNT ONWHILE @@FETCH_STATUS = 0BEGIN -- 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 ENDnext_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, @shipmentDateENDCLOSE allOrdersDEALLOCATE allOrders--select * from #testResults DROP TABLE #headers DROP TABLE #parentRoll |
 |
|
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=42516My 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 KizerMicrosoft MVP for Windows Server System - SQL Serverhttp://weblogs.sqlteam.com/tarad/Subscribe to my blog |
 |
|
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! |
 |
|
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 |
 |
|
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.aspxfor 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.- Jeffhttp://weblogs.sqlteam.com/JeffS |
 |
|
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 |
 |
|
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! |
 |
|
|
|
|
|
|