In a recent discussion it was suggested to me that not only is dynamic T-SQL useful for things like catch-all queries or some really hard to solve problems involving variable table lists, but is, in fact, a perfectly acceptable design pattern for all queries against a database. Note, in this case, we’re not talking about an ORM tool which takes control of the system through parameterized queries, but rather an intentional choice to build nothing but dynamic T-SQL directly on the system.
To me, this was immediately problematic. I absolutely agree, you’re going to have dynamic T-SQL for some of those odd-ball catch-all search queries. But to simply expand that out to include all your queries is nuts. There really is a reason that stored procedures exist, and it’s not to build dynamic T-SQL. First things first, we are talking about using sp_executesql so we can avoid problems with SQL Injection, although that should be the very first concern that comes from this methodology. But after that, you need to worry about your management of the system. Here’s an example query:
CREATE PROCEDURE [dbo].[SearchRecords] @searchQuery AS NVARCHAR(100), @col AS VARCHAR(100)Â Â Â AS BEGIN Â Â Â SET NOCOUNT ON; Â Â Â DECLARE @SQL NVARCHAR(1000); Â Â Â DECLARE @value NVARCHAR(100); Â Â Â SET @value = @searchQuery; Â Â Â IF (@col = 'PERSON_ID') Â Â Â Â Â Â Â BEGIN Â Â Â Â Â Â Â Â Â Â Â SET @SQL = 'SELECT * FROM dbo.Movie WHERE MovieId = @value'; Â Â Â Â Â Â Â END Â Â Â ELSE IF (@col = 'FIRST_NAME') Â Â Â Â Â Â Â BEGIN Â Â Â Â Â Â Â Â Â Â Â SET @SQL = 'SELECT * FROM dbo.Movie WHERE MovieName = @value'; Â Â Â Â Â Â Â END Â Â Â EXEC sp_executesql @SQL,N'@value nvarchar(20)',@value=@value; END GO
No chance of SQL injection with this, but there are other problems. The first one that comes up for me is that I’ve had to use a generic data type for @value, NVARCHAR(100). I’m passing that to both an ID and a NVARCHAR which means, when passing it to the ID I’m going to get a implicit conversion, possibly affecting index use for that part of the query. But it gets worse. Let’s execute the query twice, but I want clear the cache (please only do this on test systems):
DBCC FREEPROCCACHE(); GO EXEC dbo.SearchRecords @searchQuery = N'42', -- nvarchar(100) @col = 'PERSON_ID' -- varchar(100); GO EXEC dbo.SearchRecords @searchQuery = N'Serpico', -- nvarchar(100) @col = 'FIRST_NAME' -- varchar(100); GO
So now we have two different plans in cache. If I query the cache like this:
SELECTÂ Â Â deqp.query_plan, deqs.execution_count, deqs.query_hash, deqs.query_plan_hash, dest.text, OBJECT_NAME(dest.objectid) FROMÂ Â Â sys.dm_exec_query_stats AS deqs CROSS APPLY sys.dm_exec_sql_text(deqs.sql_handle) AS dest CROSS APPLY sys.dm_exec_query_plan(deqs.plan_handle) AS deqp; GO
I get a set of results that looks like this:
Note the lack of of an OBJECT_NAME. This is because there is no objectid stored with the plan created by sp_executesql. So, if I had hundreds of procedures that generated hundreds or even thousands of different queries through a dynamic process like this, I have absolutely no way of know which of the hundreds of procedures generated which query without going through and doing text searches against the code. I have to worry about SQL injection and I’m going to be doing all kinds of crazy searches to find the code that I need to tune or modify? No, for me, this is a very poor design pattern and not one I would suggest people adopt.
1) Please use AdventureWorks for your examples so everyone can follow along.
2) Isn’t this a better implementation, to get the correct datatypes into play??
IF (@col = ‘PERSON_ID’)
BEGIN
SET @SQL = ‘SELECT * FROM dbo.Movie WHERE MovieId = @value’;
EXEC sp_executesql @SQL,N’@value int’,@value=@value;
END
ELSE
IF (@col = ‘FIRST_NAME’)
BEGIN
SET @SQL = ‘SELECT * FROM dbo.Movie WHERE MovieName = @value’;
EXEC sp_executesql @SQL,N’@value varchar(50)’,@value=@value;
END
3) I think this type of coding is likely best done in the middle tier, but maybe my stab does take care of the extra cached items…
Hey Kevin,
1) Tough 😉
2) Could be, but I’m actually using the example code that was part of the discussion, not trying to build a better mouse trap. Yours is improved.
3) Maybe yes, maybe no. I’m just not a fan of 100% dynamic, even if you were using an ORM tool, because of a few of the things I showed here.
[…] Is Dynamic T-SQL a Good Design Pattern? - Grant Fritchey (Blog|Twitter) […]
Nice article.
I have question that what is the exact solution of the catch-all queries.
Should I use dynamic design pattern for catch-all queries and avoid this design pattern for all others queries.
Thanks.
i think it really comes down to execution time. yes, dynamic sql has some drawbacks, but the wall-time it can take to parse and cache and run dynamic sql can be far less time than for some of the WITH() queries i’ve seen (think memory allocation, no indexes, etc) – orders of magnitude in most cases. the implicit conversion can be addressed in having multiple input parameters of the proper types and using them appropriately to build the sql string to be executed.
Grant, thank you for the article. I once fell into the trap of thinking that queries I was seeing when analyzing a production workload with sys.dm_exec_query_stats were adhoc queries, ie. not issued from stored procedures, because no objectids were showing. I confronted the developer asking him why his application was executing adhoc queries and not stored procedures when connecting to the database. He said it did use procedures, but with dynamic SQL. Now I better understand how this works.
Another, in my mind, big issue with dynamic SQL is “messy” security. It is no longer enough to grant users EXEC permissions on the stored procedure. Users need to be granted explicit permissions on the tables that are part of the dynamic-SQL statement. One of the core advantages of stored procedures as an extra protection layer shielding data in the database from arbitrary access is gone. This I think is a serious threat to data security and suggests that use of dynamic SQL should indeed be an exception not the rule in database design
@Marios
I agree totally regarding security: sp_executesql wrecks the inherent good security model of stored procedures.
To Azhar,
Those crazy reporting queries that we all end up writing where you get to search on anything at any time are best served by this type of ad hoc SQL. You’re 100% on the right track.
Marios & Stephanie,
Exactly. I didn’t even cover the security aspects in this short post, but that’s another excellent point. Everything has to be much less secure in order for this type of design to work.
Not sure why everything has to be less secure. You can execute the dynamic sql for these open-ended search queries as a user that has only read permissions to the necessary tables and all DML access revoked.
Hey Kevin,
Yeah, that’s mostly true, but you’re still unzipping more there than you did previously.