Logical thinking could get very complicated

Tomz_KV

Here is a simple stored procedure in company database that does a search and returns results in less than a second:

CREATE PROCEDURE [dbo].[WorkOrder_Search](@Description varchar(250))
AS
Begin
SELECT Order_id,Dept_Code,Graphic_Number,[Description],Quantity,Date_Received as [Recv. Date],Date_Due as [Due Date],
Date_Delivered as [Delivered Date],proof_sent_date as [Proof Date],print_date as [To Print],
dbo.func_grOrder_GetAssigned(Order_id) as [Assign To],Status,Note, Master_Cal_ID
From grGraphic_Order
where [Description] like '%' + @Description + '%'
end

But it used to be a really "BIG" stored procedure as shown below that did the same search and took about 5 minutes to return results. It did return the same results as the above does.

CREATE PROCEDURE [dbo].[WorkOrder_search](@Description varchar(250))
AS
begin
SET QUOTED_IDENTIFIER OFF
SET ANSI_NULLS OFF
SET NoCount ON

Declare @sql varchar(8000)
Declare @Orig\_1 varchar(1000)
Declare @Declare varchar(200),
@From varchar(100),@Into varchar(50),@Groupby varchar(300),
@Orderby varchar(400),@Select varchar(400),@Select1 varchar(400),@Select2 varchar(300),
@cursor1 varchar (500),@cursor2 varchar(4000),@create varchar(4000)

set @Orig\_1 = ''

Create table #Assignto\_temp(Order\_id int, Assignto\_names varchar(500))

Insert into #Assignto\_temp(Order\_id )

Select gro.order\_id
From grassignto gro
Inner join wfpmauser wf
On gro.user\_id = wf.user\_id
group by gro.order\_id

declare @order\_number int, @Assignto varchar(255), @List\_names varchar(255),@order\_number\_prev int
declare assignto Cursor
Local
Forward\_only
for

Select gro.order\_id,Wf.First\_name+' '+Wf.Last\_name as Assignto
From grassignto gro
Inner join wfpmauser wf
On gro.user\_id = wf.user\_id
order by order\_id 

open assignto
Fetch Next from assignto into @order\_number, @Assignto
While @@Fetch\_Status=0
begin
		select @List\_names=Assignto\_names from #Assignto\_temp where order\_id=@order\_number
 	
	if @List\_names is null 
	begin
		update #Assignto\_temp
		set Assignto\_names=@Assignto
		where order\_id=@order\_number
	end
	else
	begin
		update #Assignto\_temp
		set Assignto\_names=@list\_names + ',' + @Assignto
		where order\_id=@order\_number
	end
	  Fetch Next from assignto into @order\_number, @Assignto
end
close assignto
deallocate assignto

Set @Declare ="Declare @Orig\_1 varchar(1000),@Originator varchar(100),@order\_id int,@Orig varchar(300) set @Ori

Brisingr Aerowing

pssssst. Use pre blocks to make your code pretty.

Gryphons Are Awesome! ‮Gryphons Are Awesome!‬

OriginalGriff

Pretty? Pretty? You'd need a very large magnet and a flare gun to make that code pretty! :laugh:

The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)

ENOTTY

Obviously the old code is a hideous display of incompetence what with the gratitious use of cursors (not even FAST_FORWARD at that) and temp tables in dynamic SQL no less, but I think the new code is missing an ORDER BY Graphic_number ASC, Dept_code DESC (with matching nonclustered indexes on the table) and probably could be improved by an inline table valued function with a CROSS APPLY in place of dbo.func_grOrder_GetAssigned(Order_id) and a FULLTEXT index for the Description column to avoid the non-SARGable wildcard prefix.

Bernhard Hiller

And now? When your boss tells you that the application is slow, where will you find some lines for easy improvements?

Tomz_KV

Great advice.

TOMZ_KV

Tomz_KV

What would you suggest to do?

TOMZ_KV

ENOTTY

Sprinkle WAITFOR DELAY '00:00:01' (or a more appropriate value, do a performance analysis first; it should run faster than the old unoptimized version) judiciously across your newly optimized SPs, so you still have room for improvement.

YDaoust

How can you be sure that both procedures are doing the same? I mean in all cases, not just on a few test cases.

Tomz_KV

The old one had been in production for a long while. Users knew that it returned correct results but just took too much time to run. They got timeout messages from time to time. Instead of fixing it, it was easier and quicker to write a new one.

TOMZ_KV