views:

1279

answers:

7

The DBA here at work is trying to turn my straightforward stored procs into a dynamic sql monstrosity. Admittedly, my stored procedure might not be as fast as they'd like, but I can't help but believe there's an adequate way to do what is basically a conditional join.

Here's an example of my stored proc:

SELECT 
*
FROM
table
WHERE
(
    @Filter IS NULL OR table.FilterField IN 
    (SELECT Value FROM dbo.udfGetTableFromStringList(@Filter, ','))
)

The UDF turns a comma delimited list of filters (for example, bank names) into a table.

Obviously, having the filter condition in the where clause isn't ideal. Any suggestions of a better way to conditionally join based on a stored proc parameter are welcome. Outside of that, does anyone have any suggestions for or against the dynamic sql approach?

Thanks

+5  A: 

Take a look here Dynamic Search Conditions in T-SQL

SQLMenace
Doesn't this conclusion seem to contract him at http://www.sommarskog.se/dynamic_sql.html#List
Brian Hasden
+4  A: 

You could INNER JOIN on the table returned from the UDF instead of using it in an IN clause

Your UDF might be something like

CREATE FUNCTION [dbo].[csl_to_table] (@list varchar(8000) )
RETURNS @list_table TABLE ([id] INT)
AS
BEGIN
    DECLARE  @index INT,
      @start_index INT,
      @id INT

    SELECT @index = 1 
    SELECT @start_index = 1
    WHILE @index <= DATALENGTH(@list)
    BEGIN

     IF SUBSTRING(@list,@index,1) = ','
     BEGIN

      SELECT @id = CAST(SUBSTRING(@list, @start_index, @index - @start_index ) AS INT)
      INSERT @list_table ([id]) VALUES (@id)
      SELECT @start_index = @index + 1
     END
     SELECT @index  = @index + 1
    END
    SELECT @id = CAST(SUBSTRING(@list, @start_index, @index - @start_index ) AS INT)
    INSERT @list_table ([id]) VALUES (@id)
    RETURN
END

and then INNER JOIN on the ids in the returned table. This UDF assumes that you're passing in INTs in your comma separated list

EDIT:

In order to handle a null or no value being passed in for @filter, the most straightforward way that I can see would be to execute a different query within the sproc based on the @filter value. I'm not certain how this affects the cached execution plan (will update if someone can confirm) or if the end result would be faster than your original sproc, I think that the answer here would lie in testing.

Russ Cam
But what if the @Filter is null and not passed in? Wouldn't that cause the stored proc to return no data.
Brian Hasden
+2  A: 

Looks like the rewrite of the code is being addressed in another answer, but a good argument against dynamic SQL in a stored procedure is that it breaks the ownership chain.

That is, when you call a stored procedure normally, it executes under the permissions of the stored procedure owner EXCEPT when executing dynamic SQL with the execute command,for the context of the dynamic SQL it reverts back to the permissions of the caller, which may be undesirable depending on your security model.

In the end, you are probably better off compromising and rewriting it to address the concerns of the DBA while avoiding dynamic SQL.

JohnFx
Thanks John. That's good information. Any ideas on how to accomplish this?According to Erland at http://www.sommarskog.se/dyn-search-2005.html he seems to suggest dynamic sql is indeed the way to go, yet at http://www.sommarskog.se/dynamic_sql.html#List he suggests otherwise.Thanks.
Brian Hasden
+2  A: 

I am not sure I understand your aversion to dynamic SQL. Perhaps it is that your UDF has nicely abstracted away some of the messyness of the problem, and you feel dynamic SQL will bring that back. Well, consider that most if not all DAL or ORM tools will rely extensively on dynamic SQL, and I think your problem could be restated as "how can I nicely abstract away the messyness of dynamic SQL".

For my part, dynamic SQL gives me exactly the query I want, and subsequently the performance and behavior I am looking for.

RedFilter
When you use dynamic sql you lose all "compile time" checking. Everything becomes a "run time" error. That's the big problem I have. Even with the DBAs "new and improved" dynamic sql stored proc it's having "run time" errors.
Brian Hasden
dynamic SQl is also far less secure. It requires you to set the permissions at the table level and not at the stored proc level. Therefore users have direct access to the table instead of being limited to doing only the things in the proc. This leads to a situation where fraud is easy to commit.
HLGEM
@Brian - that's case of proper validation of user input. Users should enever be supplying table or view names as part of their data input.@HLGEM - that's a good point. In my case, I escalate mgmt of permissions to the application level, but in a pure SQL app that is not possible
RedFilter
+2  A: 

I don't see anything wrong with your approach. Rewriting it to use dynamic SQL to execute two different queries based on whether @Filter is null seems silly to me, honestly.

The only potential downside I can see of what you have is that it could cause some difficulty in determining a good execution plan. But if the performance is good enough as it is, there's no reason to change it.

Dave Costa
I'm told it isn't performing well enough.
Brian Hasden
+1  A: 

No matter what you do (and the answers here all have good points), be sure to compare the performance and execution plans of each option.

Sometimes, hand optimization is simply pointless if it impacts your code maintainability and really produces no difference in how the code executes.

I would first simply look at changing the IN to a simple LEFT JOIN with NULL check (this doesn't get rid of your udf, but it should only get called once):

SELECT *
FROM table
LEFT JOIN dbo.udfGetTableFromStringList(@Filter, ',') AS filter
    ON table.FilterField = filter.Value
WHERE @Filter IS NULL
    OR filter.Value IS NOT NULL
Cade Roux
+1  A: 

It appears that you are trying to write a a single query to deal with two scenarios:
1. @filter = "x,y,z"
2. @filter IS NULL

To optimise scenario 2, I would INNER JOIN on the UDF, rather than use an IN clause...

SELECT * FROM table
INNER JOIN dbo.udfGetTableFromStringList(@Filter, ',') AS filter
  ON table.FilterField = filter.Value

To optimise for scenario 2, I would NOT try to adapt the existing query, instead I would deliberately keep those cases separate, either an IF statement or a UNION and simulate the IF with a WHERE clause...

TSQL IF

IF (@filter IS NULL)
  SELECT * FROM table
ELSE
  SELECT * FROM table
  INNER JOIN dbo.udfGetTableFromStringList(@Filter, ',') AS filter
    ON table.FilterField = filter.Value


UNION to Simulate IF

SELECT * FROM table
  INNER JOIN dbo.udfGetTableFromStringList(@Filter, ',') AS filter
    ON table.FilterField = filter.Value

UNION ALL

SELECT * FROM table WHERE @filter IS NULL

The advantage of such designs is that each case is simple, and determining which is simple is it self simple. Combining the two into a single query, however, leads to compromises such as LEFT JOINs and so introduces significant performance loss to each.

Dems