Is this stored procedre optimised ?

Hi i am using following stored procedure on MS SQL 2000 server & also on MS SQL 2005
Its working fine.
Now for better performance (related to execution speed , memory , recompilation problems)
Is this stored procedure optimized or needs to be optimize?
if it require to write in more better manner how can i modify?
is any suggestions?
thanks in advance...


SET QUOTED_IDENTIFIER ON
GO
SET ANSI_NULLS ON
GO

if exists (select * from dbo.sysobjects where id = object_id(N'[dbo].[cmdb_device_Paging]') and OBJECTPROPERTY(id, N'IsProcedure') = 1)
drop procedure [dbo].[cmdb_device_Paging]
GO
CREATE PROCEDURE cmdb_device_Paging
(
@criteria varchar(15),
@custID varchar(20),
@pageSize varchar(5),
@topValue varchar(5)

)
AS
SET NOCOUNT ON
if @criteria = 'device'
begin
exec (
'SELECT top '+@pageSize+ ' dev.DeviceId,dev.Address,dev.DeviceType,dev.MI,dev.DeviceName,dev.Hardware,
dev.Software,dev.Community,dev.operation FROM Accounts ac INNER JOIN Device_PlaceHolder dev
ON ac.SiteID = dev.SiteID AND ac.CustID = '''+@custID+'''' +' where dev.DeviceID not in( SELECT top '+@topValue+
' dev.DeviceId FROM Accounts ac INNER JOIN Device_PlaceHolder dev ON ac.SiteID = dev.SiteID
AND ac.CustID = '''+@custID+''''+' )'
)
end

if @criteria = 'managedDev'
begin
exec (
'SELECT top '+@pageSize+ ' dev.DeviceID ,dev.Address ,dev.DeviceType ,dev.MI,dev.DeviceName ,dev.Hardware ,dev.Software
,dev.Community ,dev.Operation FROM Accounts ac INNER JOIN Device_PlaceHolder dev ON ac.SiteID = dev.SiteID AND
ac.CustID = '''+@custID+'''' +' AND dev.operation IN('+'''MAD'''+','+'''MS'''+')'+' where dev.DeviceID not in ( SELECT top '+@topValue+
' dev.DeviceID FROM Accounts ac INNER JOIN Device_PlaceHolder dev ON ac.SiteID = dev.SiteID
AND ac.CustID = '''+@custID+''''+' AND dev.operation IN('+'''MAD'''+','+'''MS'''+')'+ ')'
)

end

if @criteria = 'unManagedDev'
begin
exec (
'SELECT top '+@pageSize+ ' dev.DeviceID ,dev.Address ,dev.DeviceType ,dev.MI,dev.DeviceName ,dev.Hardware ,dev.Software
,dev.Community ,dev.Operation FROM Accounts ac INNER JOIN Device_PlaceHolder dev ON ac.SiteID = dev.SiteID AND
ac.CustID = '''+@custID+'''' +' AND dev.Operation = '+ '''U'''+' where dev.DeviceID not in ( SELECT top '+@topValue+
' dev.DeviceID FROM Accounts ac INNER JOIN Device_PlaceHolder dev ON ac.SiteID = dev.SiteID
AND ac.CustID = '''+@custID+'''' +' AND dev.Operation = '+ '''U'''+' )'
)
end

if @criteria = 'IgnoreDev'
begin
exec (
'SELECT top '+@pageSize+ ' dev.DeviceID ,dev.Address ,dev.DeviceType ,dev.MI,dev.DeviceName ,dev.Hardware ,dev.Software
,dev.Community ,dev.Operation FROM Accounts ac INNER JOIN Device_PlaceHolder dev ON ac.SiteID = dev.SiteID AND
ac.CustID = '''+@custID+'''' +' AND dev.Operation = '+ '''I'''+' where dev.DeviceID not in ( SELECT top '+@topValue+
' dev.DeviceID FROM Accounts ac INNER JOIN Device_PlaceHolder dev ON ac.SiteID = dev.SiteID
AND ac.CustID = '''+@custID+'''' +' AND dev.Operation = '+ '''I'''+' )'

)
end
GO
SET QUOTED_IDENTIFIER OFF
GO
SET ANSI_NULLS ON
GO

Why are you using dynamic sql when the only thing you have variable in the query is the @top and the one predicate ?

You can use a @top variable with sql2005 !!! (NEW)

So make all your queries parameterized queries using the @top-variable and get rid of the dynamic sql !

e.g.
SELECT top ( @pageSize )
dev.DeviceId,dev.Address,dev.DeviceType,dev.MI,dev.DeviceName,dev.Hardware,
dev.Software,dev.Community,dev.operation
FROM Accounts ac
INNER JOIN Device_PlaceHolder dev
ON ac.SiteID = dev.SiteID A
AND ac.CustID = @custID
where dev.DeviceID not in( SELECT top ( @topValue ) dev.DeviceId
FROM Accounts ac
INNER JOIN Device_PlaceHolder dev
ON ac.SiteID = dev.SiteID
AND ac.CustID = @custID )

hi
thanks for reply, whatever you suggest its fine & i will give it try
BUT
When i check in more detail then found that in each query i am using " NOT IN "
when topValue in inner query increased more than 1 lac then it will very very slow.
So now i want to remove NOT IN with IN or have to use CURSUR so how can i do that?

thaks in advance...

AVOID cursors wherever you can and whenever you can !

Convert it to a NOT EXISTS ...

where NOT EXISTS( SELECT * FROM Accounts ac1
INNER JOIN Device_PlaceHolder dev1
ON ac1.SiteID = dev1.SiteID
AND ac1.CustID = @custID
and dev1.DeviceID = dev.DeviceID )

btw it has the intelligence not doing more than needed when you use "exists"

Hi,
TOP can be parameterised in 2005 but is not compatible with 2000.
You can consider this approach as well which will work with bith 2000 and 2005.

SET ROWCOUNT @N
SELECT QUERY
SET ROWCOUNT 0

hi
thanks for instant reply, its fine
BUT
if i rplace NOT IN with NOT EXIST then also it will doing same --like 1st it will execute inner query and then it will compare primary key whether it is present in ids obtained in inner query .
am i right?

thaks in advance...

but also keep in mind BOL states "SET ROWCOUNT " still works but is obsolete !

hi alzdba
thanks for instant reply, its fine
BUT
if i rplace NOT IN with NOT EXIST then also it will doing same --like 1st it will execute inner query and then it will compare primary key whether it is present in ids obtained in inner query .
am i right?

thaks in advance...

With the not exists, I've added the extra line, making it a correlated subquery. Basicaly telling sqlserver to first crate a workset and then check its keys to the correlated subquery.
There should be an index on deviceid to speed it up , or the index for custid should filter very most of the resultset.

It should be faster than the "not in" clause or a "left join".

thanks
ok fine u r right i have indexed column on deviceid but then when i run sample query given below then it giving syntax error -
Msg 156, Level 15, State 1, Line 2
Incorrect syntax near the keyword 'EXISTS'.

select top 50 * from device_placeholder where deviceid
NOT EXISTS(select top 20000 deviceid from device_placeholder)

missed to mention that i am using SQL server 2000

thanks
ok fine u r right i have indexed column on deviceid but then when i run sample query given below on SQL server 2000 ,then it giving syntax error -
Msg 156, Level 15, State 1, Line 2
Incorrect syntax near the keyword 'EXISTS'.


select top 50 * from device_placeholder where deviceid
NOT EXISTS(select top 20000 deviceid from device_placeholder)

- If you only use SQL2000, you've posted in the wrong forum (SQL2005)

With sql2000 the top clause in not dynamic !!
But in that case I would implement a maximum top-clause and eventualy only show a dynamyc top-x in the application.


-- for you exists query :
select top 50 * from device_placeholder dev
where deviceid
NOT EXISTS ( SELECT * FROM Accounts ac1
INNER JOIN Device_PlaceHolder dev1
ON ac1.SiteID = dev1.SiteID
AND ac1.CustID = @custID
and dev1.DeviceID = dev.DeviceID )


-- the 'select *' being used in the 'NOT EXISTS' part will only check for existance of a row. It will not fetch all columns for all rows that match the condition !! so this action is ended when the first occurence is found )