March 22, 2011 at 4:24 pm
I came across this link curtesy of the SSC newsleter ... http://www.sqlteam.com/article/using-dynamic-sql-in-stored-procedures ... essentially this is the bit of dynamic SQL and I've seen heaps of these ...
CREATE PROCEDURE [Sales].[GetSalesOrders] (
@CustomerID INT = NULL,
@ContactID INT = NULL,
@debug bit = 0 )
AS
DECLARE @SQL NVARCHAR(4000);
DECLARE @ParameterDefinition NVARCHAR(4000);
SELECT@ParameterDefinition = '
@CustomerParameter INT,
@ContactParameter INT
';
SELECT@SQL = N'
SELECT[SalesOrderID], [OrderDate], [Status],
[CustomerID], [ContactID]
FROM[Sales].[SalesOrderHeader]
WHERE 1 = 1
';
IF @CustomerID IS NOT NULL
SELECT @SQL = @SQL + N'
AND CustomerID = @CustomerParameter ';
IF @ContactID IS NOT NULL
SELECT @SQL = @SQL + N'
AND ContactID = @ContactParameter ';
IF @debug = 1
PRINT @SQL
EXEC sp_executeSQL
@SQL,
@ParameterDefinition,
@CustomerParameter = @CustomerID,
@ContactParameter = @ContactID;
But wouldn't this be simplier and work just as well?
CREATE PROCEDURE [Sales].[GetSalesOrders] (
@CustomerID INT = NULL,
@ContactID INT = NULL
as
SELECT[SalesOrderID], [OrderDate], [Status],
[CustomerID], [ContactID]
FROM[Sales].[SalesOrderHeader]
WHERE 1 = case
when @CustomerID is null then 1 /* no param so return everything */
when @CustomerID = CustomerId then 1 /* equals param so return */
else 0 end /* doesn't equal param so skip */
and 1 = case
when @ContactID is null then 1 /* no param so return everything */
when @ContactID = @ContactID then 1 /* equals param so return */
else 0 end /* doesn't equal param so skip */
OPTION (RECOMPILE);
Thoughts?
Editted: Opps. Sorry, left out the OPTION (RECOMPILE);
March 22, 2011 at 4:36 pm
In general dynamic sql is a bad idea and you should avoid it...however your example code is building "catch all query". Have a look at this article:
http://sqlinthewild.co.za/index.php/2009/03/19/catch-all-queries/[/url]
In your case the dynamic sql may be less readable but may perform much better than using a CASE expression in the WHERE clause.
There are no special teachers of virtue, because virtue is taught by the whole community.
--Plato
March 22, 2011 at 4:38 pm
Your solution is termed a "catch-all query". Read this blog post[/url] by MVP Gail Shaw on why you shouldn't do this... and why the dynamic sql is actually a better solution.
Another solution is to break this up, but those two optional parameters will "blow" up this one procedure to 4:
change the existing procedure to:
if @parameter1 is not null and @parameter2 is not null execute ProcBothParameters @Parameter1, @Parameter2
else if @Parameter1 is not null execute ProcParameter1 @Parameter1
else if @Parameter2 is not null execute ProcParameter2 @Parameter2
where each of other procedures runs the query as appropriate for the parameter(s) being passed in.
Additional parameters will blow this up even more - exponentially!
Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes
March 22, 2011 at 4:40 pm
... and opc.three beat me to it again... - even posted the same blog link!
Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes
March 22, 2011 at 4:46 pm
Jinx! Diet Coke please Wayne 😎
Just passing along the knowledge...Gail's article is the de facto standard on the topic.
There are no special teachers of virtue, because virtue is taught by the whole community.
--Plato
March 22, 2011 at 4:51 pm
I have one situation where I couldn't find any other way of returning the data I wanted.
I have a lot of tables (at least a hundred) with a lot of datetime columns (about 30 in each table).
The application is all about processes that have to run to specific timelines. Each table represents a stage in the timeline and each field represents an action that has to take place on a certain date.
Every one of the datetime fields is recorded in tblFields which gives each field an ID and records the field name and the name of the table it belongs to.
There is also tblFieldRelationships that contains BaseFieldID and CalcFieldID and the designated interval, in days, between them.
When I an writing procedures that return the data for a stage I pass each fieldname in that stage's table to a function that uses a bit of dynamic sql - it takes the fieldname passed in, finds out the base field and returns the calculated date. It's a bit of a palaver to set up tblFields and tblFieldRelationships but I couldn't think of any easier way to say ... 'this field has a specified relationship with another field - what date should this field be?' (bearing in mind the base date changes like a yo-yo)
March 22, 2011 at 4:57 pm
Just as a matter of curiousity ... I know I'll get shot down in flames ... but is what is posted above different from ...
SELECT [SalesOrderID], [OrderDate], [Status],
[CustomerID], [ContactID]
FROM [Sales].[SalesOrderHeader]
WHERE (CustomerID = @CustomerID OR @CustomerID IS NULL)
AND (ContactID = @ContactID OR @ContactID IS NULL)
March 22, 2011 at 5:04 pm
sku370870 (3/22/2011)
Just as a matter of curiousity ... I know I'll get shot down in flames ... but is what is posted above different from ...
SELECT [SalesOrderID], [OrderDate], [Status],
[CustomerID], [ContactID]
FROM [Sales].[SalesOrderHeader]
WHERE (CustomerID = @CustomerID OR @CustomerID IS NULL)
AND (ContactID = @ContactID OR @ContactID IS NULL)
It is different, and delivers a less costly execution plan than the query using the CASE statement in the example, but the technique in general will still deliver more costly plans than the dynamic SQL -or- IF...ELSE approaches where no NULL checks are being done in the WHERE clause.
There are no special teachers of virtue, because virtue is taught by the whole community.
--Plato
March 22, 2011 at 5:11 pm
Sorry guys. Left out the OPTION (RECOMPILE) in my haste. Added it in. Actually, in many situations you actually find through your performance testing with multiple parameter combinations (which of course we all do) it's not required and the optimiser gets it right without forcing a recompile.
My point, which I perhaps didn't make all that clearly, is that there isn't a "one size fits all" for dynamic queries and sometime keeping things simple makes for faster development and more maintainable code.
Sure, the expense might be a few extra CPU cycles or a few milli-seconds added to a query but servers are dead cheap nowadays and developers are much more expensive.
March 22, 2011 at 5:37 pm
belgarion (3/22/2011)
Sorry guys. Left out the OPTION (RECOMPILE) in my haste. Added it in. Actually, in many situations you actually find through your performance testing with multiple parameter combinations (which of course we all do) it's not required and the optimiser gets it right without forcing a recompile.
In general the optimizer has improved at making decisions from release to release. It's not the times when it picks right however that we need to defend against...enter OPTION (RECOMPILE), IF...ELSE and dynamic SQL.
belgarion (3/22/2011)
My point, which I perhaps didn't make all that clearly, is that there isn't a "one size fits all" for dynamic queries and sometime keeping things simple makes for faster development and more maintainable code.Sure, the expense might be a few extra CPU cycles or a few milli-seconds added to a query but servers are dead cheap nowadays and developers are much more expensive.
As usual...it will depend on your situation. I have seen the IF...ELSE approach outperform the other two methods. I have also seen architects implement an obscene amount of UDFs so they could have clean looking, easily maintainable code bases. When playing the hand blind (to use a poker reference) I bet that the optimizer will do best with dynamic SQL in more situations than the others and will take the hit on maintenance and readability.
There are no special teachers of virtue, because virtue is taught by the whole community.
--Plato
March 22, 2011 at 6:30 pm
I'm on the fence on catch-all queries, and yes, I know the optimizer problems. It doesn't mean I like dynamic SQL in general.
I have a simple rule when it comes to things like that. If it's not breaking, leave it the heck alone, and leave it easier to read/maintain.
The IF/ELSE structure actually can behave worse then the catchall query if you don't use a wrapper proc to call multiple unique procs, like I've seen in some places (IE If @p1 is null begin select end if @p2 is null begin select end...)
I'll typically use dynamic SQL for only a couple of things. Dynamic Pivots (which screw up other things, like the expected results, so be wary and have a good coder handy), multiple database processing where I don't want to run for all dbs, or a single piece of code that I want to run for multiple tables in succession.
Dynamic SQL has it's uses, but it's a final solution for me. I prefer to avoid it when possible, but then, I avoid running 'execute as' too. When maintaining the dynamic SQL is easier then maintaining the process any other way, then it suddenly becomes your best friend though.
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
March 23, 2011 at 8:03 am
belgarion (3/22/2011)
Sorry guys. Left out the OPTION (RECOMPILE) in my haste. Added it in.
Be careful with that - you need to be on SQL 2008 SP1 CU5+ (I think I've got this right) for it to work properly. There were issues with it, and it was reverted back to the older 2005 method for a while until they got it fixed and tested.
Edit: Maybe Gail will see this and respond - I know she was looking into this recently.
Wayne
Microsoft Certified Master: SQL Server 2008
Author - SQL Server T-SQL Recipes
March 23, 2011 at 8:24 am
opc.three (3/22/2011)
In general dynamic sql is a bad idea and you should avoid it...
In general, I disagree. 😀 There are some very easy steps that can be taken to prevent SQL Injection and to make execution plans reusable.
Heh... now... if you had said "In general, EMBEDDED SQL is a bad idea", then I'd come closer to agreeing. 😉
I do agree that certain types of "catch all" queries are a bad thing, but not always. "It Depends". :hehe:
--Jeff Moden
Change is inevitable... Change for the better is not.
March 23, 2011 at 8:28 am
Discussed here - http://sqlinthewild.co.za/index.php/2011/03/18/q-a-from-24-hours-of-pass-session/
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
March 23, 2011 at 8:33 am
belgarion (3/22/2011)
Sure, the expense might be a few extra CPU cycles or a few milli-seconds added to a query but servers are dead cheap nowadays and developers are much more expensive.
Sorry I beg to differ, for catch-all type queries it's not a case of a few extra CPU cycles or a few milliseconds. It can be a difference of many seconds, millions of IOs and measurable % of CPU differences.
I have one client that had that query pattern all over their database. After fixing 4 commonly called queries (just changing the catch-all to dynamic SQL), the average CPU usage during business hours dropped 20% and throughput increased by an order of magnitude.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
Viewing 15 posts - 1 through 15 (of 20 total)
You must be logged in to reply to this topic. Login to reply