SQLServerCentral Article

Dos and Don'ts of Dynamic SQL

,

Dynamic SQL can be an incredibly powerful tool when used properly, however, it can an incredibly large security flaw or a pain to debug if written poorly. The below is a few of Dos (and Don’ts) that I have that may help you in the future when writing Dynamic Statements.

Don’t use Dynamic SQL if your statement isn’t Dynamic

This might seem like a silly thing to say, but countless times time I have seen examples of dynamic SQL where the query doesn’t need to be dynamic at all. Take the following procedure, for example:

CREATE PROC MyProc @ID int AS
BEGIN
    DECLARE @SQL nvarchar(MAX);
    SET @SQL = N'SELECT * FROM MyTable WHERE ID = ' + CONVERT(nvarchar(MAX),@ID);
    EXEC(@SQL);
END;

There’s actually nothing “dynamic” about this statement. @ID doesn’t need to be injected into the statement, and neither does a variable to hold the statement need to be used. You could easily replace the whole thing with a parametrised statement:

CREATE PROC MyProc @ID int AS
BEGIN
    SELECT * FROM MyTable WHERE ID = @ID;
END;

Do quote the names of your objects properly

One thing that is incredibly important when using dynamic SQL is making sure your object names are properly quoted. That means not injecting the raw value of your dynamic object straight into your dynamic statement, like in the below:

SET @SQL = N'SELECT * FROM ' + @Table;
SET @SQL = N'SELECT * FROM [' + @Table + N']';

Both of these statements are just as bad as the other; simply wrapping the dynamic object’s name in a couple of brackets does not make your code “safe” from injection. Just like with a literal string, brackets can be escaped, and someone malicious will be try to escape the value.

Fortunately SQL Server has a handy function to help keep you safer, which is QUOTENAME. Simply wrap the function round the variable with the dynamic object’s name and the function will automatically wrap the name in brackets and (just as importantly) escape any characters accordingly.

SET @SQL = N'SELECT * FROM ' + QUOTENAME(@Table);

Don’t inject values of parameters

When writing dynamic statements, it’s very likely you’re also going to need to pass the value of a parameter for the query as well. These aren’t values for a dynamic object but are likely something that is going to appear in your WHERE clause as part of a Boolean expression. The way to pass there parameters is not like the below example:

SET @SQL = N'SELECT * FROM ' + QUOTENAME(@MyTable) +
           N' WHERE ID = ' + CONVERT(nvarchar(MAX),@ID);
EXEC (@SQL);

Dynamic SQL can (and should) be parametrised just like any other SQL statement. This can be achieved by using sp_executesql instead of just executing the dynamic statement using EXEC. sp_executesql‘s second parameter is used to declare any variables that will be passed to the dynamic statement, and then the values for those variables can be passed as further parameters:

SET @SQL = N'SELECT * FROM ' + QUOTENAME(@MyTable) +
           N' WHERE ID = @ID;';
EXEC sp_executesql @SQL, N'@ID int', @ID = @ID;

This can even be used to output values as well:

SET @SQL = N'SELECT @Count = COUNT(*) FROM ' + QUOTENAME(@MyTable) + N';';
EXEC sp_executesql @SQL, N'@Count bigint OUTPUT', @Count = @Count OUTPUT;

Parametrising your Dynamic Statement has the advantage that plans can be reused as well (when the value of the dynamic object is the same). If you inject the parameter's value then the plan won't be reused. The Data Engine would create separate query plans for queries where the clause WHERE ID = 1 and WHERE ID = 2 ; however a query with WHERE ID = @IDwould use the same plan, regardless of the value of @ID.

Do take the time to format your Dynamic SQL

One common reason I hear for why people don’t use dynamic SQL is because it’s difficult to troubleshoot. I personally find this a little untrue. Poorly written and/or formatted dynamic SQL is difficult to troubleshoot, but then the same is true for poorly written SQL in general. Let’s take something that looks like a simple enough query:

DECLARE @SQL nvarchar(MAX);
SET @SQL = N'INSERT INTO dbo.Emails(Email) ' +
           STUFF((SELECT N'UNION ALL ' + 
                         N'SELECT Email '+
                         N'FROM ' + QUOTENAME(t.[name])
                  FROM sys.tables t
                       JOIN sys.columns c ON t.object_id = c.object_id
                  WHERE c.[name] = N'Email'
                  FOR XML PATH(N'')),1,9,'') + N';';
EXEC sp_executesql @SQL;

This query look well formatted, and it’s quite easy to read. The problem is that the dynamic SQL is returning an error (perhaps something about an invalid object name). Therefore, before executing the dynamic statement you print out its value, which returns something like this:

INSERT INTO dbo.Emails(Email) SELECT Email FROM [MyTable_A] UNION ALL SELECT Email FROM [MyTable_B] UNION ALL SELECT Email FROM [MyTable_C] UNION ALL SELECT Email FROM [MyTable_D] UNION ALL SELECT Email FROM [MyTable_E] UNION ALL SELECT Email FROM [MyTable_F] UNION ALL SELECT Email FROM [MyTable_G] UNION ALL SELECT Email FROM [MyTable_H] UNION ALL SELECT Email FROM [MyTable_I] UNION ALL SELECT Email FROM [MyTable_J];

Now that “well written” dynamic statement turns out to look like a complete eyesore. So, how do you fix this? My personal preference is to add a carriage returns and line breaks, as well an indents into my dynamic statements, just like I would any “normal” query. I tend to NCHAR(13) and NCHAR(10) for carriage return and line break respectively, but you can add the values however you wish. So the above you might write something like this:

DECLARE @SQL nvarchar(MAX);
SET @SQL = N'INSERT INTO dbo.Emails(Email)' + NCHAR(13) + NCHAR(10) +
           STUFF((SELECT NCHAR(13) + NCHAR(10) +
                         N'UNION ALL' + NCHAR(13) + NCHAR(10) +
                         N'SELECT Email' + NCHAR(13) + NCHAR(10) +
                         N'FROM ' + QUOTENAME(t.[name])
                  FROM sys.tables t
                       JOIN sys.columns c ON t.object_id = c.object_id
                  WHERE c.[name] = N'Email'
                  FOR XML PATH(N''),TYPE).value(N'.','nvarchar(MAX)'),1,13,'') + N';';
PRINT @SQL;

When you then print out the statement from this query you end up with something far more readable.

INSERT INTO dbo.Emails(Email)
SELECT Email FROM [MyTable_A]
UNION ALL
SELECT Email FROM [MyTable_B]
UNION ALL
SELECT Email FROM [MyTable_C]
UNION ALL
SELECT Email FROM [MyTable_D]
UNION ALL
SELECT Email FROM [MyTable_E]
UNION ALL
SELECT Email FROM [MyTable_F]
UNION ALL
SELECT Email FROM [MyTable_G]
UNION ALL
SELECT Email FROM [MyTable_H]
UNION ALL
SELECT Email FROM [MyTable_I]
UNION ALL
SELECT Email FROM [MyTable_J];

If you’re writing a more “normal” query, then the same logic applies. A query like this “looks” well formatted:

DECLARE @TableName sysname = N'MyTable';
DECLARE @SQL nvarchar(MAX);
SET @SQL = N'SELECT @TableName AS TableName,' +
           N'ID,' +
           N'[name] AS CustomerName ' + 
           N'FROM ' + QUOTENAME(@TableName) + N' ' +
           N'WHERE ID = @ID ' +
           N'AND Status = ''Active'';';
PRINT @SQL;
--EXEC sp_executesql @SQL, N'@ID int', @ID = @ID;

But, once printed out, turns into a single line of SQL, that can be hard to debug. If, however, you add some formatting into the dynamic statement as well, then it becomes much more readable. Applying the same idea, you could write the above query as:

DECLARE @TableName sysname = N'MyTable';
DECLARE @SQL nvarchar(MAX);
SET @SQL = N'SELECT @TableName AS TableName,' + NCHAR(13) + NCHAR(10) +
           N'       ID,' + NCHAR(13) + NCHAR(10) +
           N'       [name] AS CustomerName' + NCHAR(13) + NCHAR(10) +
           N'FROM ' + QUOTENAME(@TableName) + NCHAR(13) + NCHAR(10) +
           N'WHERE ID = @ID' + NCHAR(13) + NCHAR(10) +
           N'  AND Status = ''Active'';';
PRINT @SQL;
--EXEC sp_executesql @SQL, N'@ID int', @ID = @ID;

This formatting may look a little odd to some, but I've personally found it invaluable when dealing with high complexity dynamic statements. The above statements don't really lose any readability either, although may be a bit confusing to those newer to the language. But would you rather something than looks a little more complex at the generation point, or a huge unreadable mess when you print the dynamic statements value? I know I'd prefer the former.

Do perform whitelist checks to avoid errors on invalid objects and make your statements safer

Making sure you perform a whitelist process can be really important, especially if you want to avoid passing an error back to the presentation layer. With dynamic object names, this can be achieved my checking the values of the dynamic objects against the system tables, such as sys.tables or INFORMATION_SCHEMA.COLUMNS, or your own table of “allowed” values. Passing the value NULL to sp_executesql does not generate an error, but also, it won’t actually run anything; making your statement even safer from any injection attempts.

DECLARE @TableName sysname = N'MyTable]; CREATE DATABASE Test;--';
DECLARE @SQL nvarchar(MAX);
SELECT @SQL = N'SELECT *' + NCHAR(13) + NCHAR(10) +
              N'FROM ' + QUOTENAME(s.[name]) + N'.' + QUOTENAME(t.[name]) + N';'
FROM sys.tables t
     JOIN sys.schemas s ON t.schema_id = s.schema_id
WHERE t.[name] = @TableName
  AND s.[name] = N'dbo';
PRINT @SQL;
EXEC sp_executesql @SQL;

The above ends up not printing anything, nor running any dynamic SQL, as it’s going to be very unlikely you have a table with the name in this script.

Don’t use nvarchar(MAX) for your object’s name parameter

The maximum length of an object’s name in SQL Server is 128 characters, so why do you need space for 2GB worth of characters? Quite simply, you don’t. As you may have seen, I’ve been making use of the data type sysname, which is a synonym for nvarchar(128). If you know that the names of your objects are going to be shorter than 128 characters, then certainly use a smaller length, but there is never any need to have a length of more than 128 characters when dealing with SQL Server objects. I do recommend using nvarchar rather than varchar, as a object can contain Unicode characters. Just because you "know" your objects don't contain any of those characters now, doesn't mean they won't in the future (and the parameter to be implicitly cast to a nvarchar if you're white listing against the system objects as well).

Don’t always debug the code that creates the dynamic SQL first, debug the generated SQL statement instead

Continuing on my earlier point of making your SQL easier to debug by using formatting inside the dynamic statement, often it’s easier to debug the generated statement first and then the code that creates it. Using Print (or a SELECT Statement if your dynamic SQL is over 4,000 characters) you can inspect the SQL that is about to be run. You can then run that code on its own and see where the error is being generated and more importantly why. Once you work out why that’s failing you can then propagate the solution to your SQL that creates the dynamic statement.

Sometimes the reason for the error can be very simple, like a space is missing between 2 words. These can be very difficult to spot when all your code looks like a literal string. Getting the value of your dynamic statement means you can view the code as it’s going to be run, with all the fancy colours that your preferred UI provides, and will make debugging the code far easier.

Rate

4.57 (23)

You rated this post 5 out of 5. Change rating

Share

Share

Rate

4.57 (23)

You rated this post 5 out of 5. Change rating