How do I pass a variable that contains a list to a dynamic SQL query?

Question

I have a function that converts a sting list of numbers into a table of integers:

USE [IFRS_Temp]
GO
/****** Object:  UserDefinedFunction [dbo].[CSVToTable]    Script Date: 01/12/2019 3:36:10 PM ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER FUNCTION [dbo].[CSVToTable] (@InStr VARCHAR(MAX))
RETURNS @TempTab TABLE
   (id int not null)
AS
BEGIN
    ;-- Ensure input ends with comma
    SET @InStr = REPLACE(@InStr + ',', ',,', ',')
    DECLARE @SP INT
DECLARE @VALUE VARCHAR(1000)
WHILE PATINDEX('%,%', @INSTR ) <> 0 
BEGIN
   SELECT  @SP = PATINDEX('%,%',@INSTR)
   SELECT  @VALUE = LEFT(@INSTR , @SP - 1)
   SELECT  @INSTR = STUFF(@INSTR, 1, @SP, '')
   INSERT INTO @TempTab(id) VALUES (@VALUE)
END
    RETURN
END

declare @listOfIDs varchar(1000);
SET @listOfIDs = '5, 6, 7, 8, 9, 15, 28, 31, 49, 51, 59, 61'; 

select id from  [dbo].[CSVToTable] (@listOfIDs) --this code is ok5

The result is correct:

This throws an error:

exec('select id from  [dbo].[CSVToTable] ('+@listOfIDs+')') -- error

result :

Procedure or function dbo.CSVToTable has too many arguments specified.

I need the second query because my query is dynamic. Thanks

Ilyes · Accepted Answer · 2019-01-12T13:09:34.430

Simply

EXECUTE ('select id from  [dbo].[CSVToTable] ('''+@listOfIDs+''')')
        declare @listOfIDs varchar(1000);

Or, which is the better way

SET @listOfIDs = '5, 6, 7, 8, 9, 15, 28, 31, 49, 51, 59, 61'; 

EXECUTE sp_executesql N'select id from  [dbo].[CSVToTable] (@listOfIDs)',
                      N'@listOfIDs VARCHAR(1000)',
                      @listOfIDs;

Why I get this error?

Procedure or function dbo.CSVToTable has too many arguments specified.

Because you really pass too much parameters, more then needed, to understand this run this query and see what you are really pass to your function

SELECT 'select id from  [dbo].[CSVToTable] ('+@listOfIDs+')';

which will return (and this is what you really trying to execute)

select id from  [dbo].[CSVToTable] (5, 6, 7, 8, 9, 15, 28, 31, 49, 51, 59, 61)

instead of (which is what you need)

SELECT 'select id from  [dbo].[CSVToTable] ('''+@listOfIDs+''')';

Ok, but why sp_executesql is better than exec?

Simply, EXEC will forces you to concatenate all of your variables into one single string, that's the worst thing about it, and that makes your code fully open to SQL injection. See Bad Habits to Kick : Using EXEC() instead of sp_executesql, this doesn't mean that sp_executesql is 100% secure, but it allows for statements to be parameterized while EXEC() dosn't, therefore It’s more secure than EXEC in terms of SQL injection.

Finally, since you tag sql-server and you don't specify the version, I suggest that you use SPLIT_STRING() function (2016+) rathar than yours, and if you don't have 2016+ version, than create your own without using WHILE loop to gain more good performance, cause WHILE loop will perform slow, thus you should avoid it.

Examples:

score 0 · Answer 2 · answered Jan 12 '19 at 12:42

There's a few problems with your SQL, however, let's start with the following:

exec('select id from  [dbo].[CSVToTable] ('+@listOfIDs+')') -- error

The reason for the failure is because it turns the SQL into:

select id from  [dbo].[CSVToTable] (1,2,3,4)

You can very quickly see why that's a problem. Instead, pass the value as a parameter:

EXEC sp_executesql N'SELECT id FROM [dbo].[CSVToTable] (@IDs);', N'@IDs varchar(1000)', @IDs = @listOfIDs;

This'll make your code safe from injection,however,your function CSVToTable uses a WHILE,which means it's going to perform badly. If you're using SQL Server 2016+ you have access to SPLIT_SPLIT. It'll be way faster than a iterative process.

If not, I recommend having a look at DelimitedSplit8K for SQL Server 2008, or DelimitedSplit8k_lead for SQL Server 2012 & 2014. You'll find a them to be far better performance wise.

How do I pass a variable that contains a list to a dynamic SQL query?

2 Answers2

Linked