tags:

views:

169

answers:

6

I have been using this method to filter my queries:

Create PROCEDURE [dbo].[pGetTask]
    @showCompletedTasks bit = 1
    ,@showInProgressTasks bit = 1
    ,@taskID int = null
    ,@projectID int = null
    ,@applicationID int = null
    ,@clientID int = null

... Snip ...

where    
    a.clientID = isnull(@clientID, a.clientID)
    and a.applicationID = isnull(@applicationID, a.applicationID)
    and p.projectID = isnull(@projectID, p.projectID)
    and t.taskID = isnull(@taskID, t.taskID)
    and curr.complete = case @showCompletedTasks when 0 then 0 else curr.complete end
    and curr.complete = case @showInProgressTasks when 0 then 1 else curr.complete end

This actually slows my queries by 2 seconds on a 664 row result set. The SQL tuning advisor isn't much help, so I figure this is not the right way to do this. Is there a right way, besides a ton of if statements?

+1  A: 

Your best bet is to use this stored procedure to call a series of more specific procedures. You have two issues:

  1. The use of the case statement causes a table scan, which (obviously) ignores any indexes you might have
  2. Even if you break the statement out into several that are called conditionally, you'll still end up with a compiled execution plan that is specific to the first call to this procedure.

If you create specific procedures, like pGetTask_Completed and pGetTask_InProgress and call them conditionally from within this proc, you shouldn't have any issues.

Adam Robinson
do you think it's better to let my application handle the filtering?
Shawn Simon
That depends entirely on the size of your result set and what you're doing. If you'll be showing both on the screen at the same time, then I see no reason not to use client-side filtering.
Adam Robinson
Case would cause an index scan but that's cause he's working on a bit field which can't be indexed. The case won't cause a table scan though...
JoshBerke
@Shawn: You can also use IF ISNULL(@clientID) etc. conditional execution in your procedure. Make up your mind what parameters can *really* be NULL and write a few specific queries, handling the logic though T-SQL control flow statements. ISNULL() is poison for indexes.
Tomalak
+6  A: 

Assuming you have properly indexed the table that the select is on, and these fields are part of the index, my guess is that it would be the calls to isnull. I would change them to this:

(@clientID is null or a.clientID = @clientId) and ...

As for the case statements, indexes on bit fields are pointless, so there's not much to do there.

casperOne
Hrm, I think that fixed the issue. I don't know for sure but I'm going to close this.
Shawn Simon
+2  A: 

Check your indexes & statistics. That seems a little slow. The other option would be to do a dynamic query essentially build a string representing your sql and execute it using sp_ExecuteSql (or Exec statement)

Edit

You could try and combine your two cases but I doubt it will have effect on performance of the query. It would look better though...

Although I'm not sure your query is right (Which is hard to say without more info) but shouldn't there be an or clause between the cases your trying to provide two states to return and by having separate params I assume I can ask for Only Complete, Only Not Complete or both...in this case you need an Or

JoshBerke
I support this approach. Disclaimer, I have an extreme bias against stored procedures used solely for stitching up SQL. I would use some other query builder in the application space and double check your indices.
Mark Canlas
Thanks Tom how many different times can I misspell separate? At least I was consistent
JoshBerke
A: 

here is a great article on this topic:

http://www.sommarskog.se/dyn-search-2005.html

It will give you a lot of ideas to try out.

I tend to to a mix of the things to make these "search" type queries go fast. Here are some that I seem to use all the time:

  • I try to make certain search parameters required, so you can hit the index on those.

  • if possible (depends on number of rows) split up the query, using temp tables. IF you only have a few hundred ClientID values, create a #ClientID temp table. Put in the one the user wants, or all of them. You can then make this the FROM table and/or inner join other tables to this to make it much faster.

  • If you have dates that are optional, don't use anything like (@startDate is null or a.date >= @startDate). Just do something like SET @startDate=COALESCE(@startDate,'01/01/1970'). This will give you a value and eliminate using an "OR" and will use an index.

KM
A: 

casperOne's suggestion is what I would start with.

One other possibility is this:

WHERE
     (1 =
     CASE
         WHEN @client_id IS NULL THEN 1
         WHEN a.clientID = @clientID THEN 1
         ELSE 0
     END) AND
...

I found that SQL Server (at least 2005) using the CASE statement like this can cause the query plan to short-circuit the rest of the logic. In the case of simple comparisons that's not really a big problem, but if your logic includes a subquery or some other expensive operation it might be a big help to short-circuit it. In your example, I would just go with casperOne's suggestion though.

Also, if you use the CASE method above, you'll need to add the RECOMPILE options to your SELECT and your stored procedure.

Tom H.
+1  A: 

You could be the victim of "parameter sniffing" problem. MS-SQL will take the parameters of your 1st run of your SP as the best sampling for making the query plan. Your query could be slow due to this.

To prove, try to run the content of your query directly, by simulating the populated parameters as variables. If it is much faster, then you are indeed having the "parameter sniffing" problem.

The solution is to trick the MS-SQL to think that your parameters are only being used to be assigned to another variables. Example:


create proc ManyParams
(
    @pcol1 int,
    @pcol2 int,
    @pcol3 int
)
as
declare
    @col1 int,
    @col2 int,
    @col3 int

select
    @col1 = @pcol1,
    @col2 = @pcol2,
    @col3 = @pcol3

select 
    col1,
    col2,
    col3
from 
    tbl 
where 
    1 = case when @col1 is null then 1 else case when col1 = @col1 then 1 else 0 end end
and 1 = case when @col2 is null then 1 else case when col2 = @col2 then 1 else 0 end end
and 1 = case when @col3 is null then 1 else case when col3 = @col3 then 1 else 0 end end

Irawan Soetomo
I agree with CasperOne. Hence, my WHERE clause should be simplified as: (@col1 is null or @col1 = col1)and (@col2 is null or @col2 = col2)and (@col3 is null or @col3 = col3)
Irawan Soetomo
Or even this: col1 = isnull(@col1, col1)and col2 = isnull(@col2, col2)and col3 = isnull(@col3, col3)
Irawan Soetomo