Skip to content

SQL Good Idea/Bad Idea

30 October 2009

The following is a SQL function that splits the string provided as an argument into a table of values on the comma character:

CREATE FUNCTION Split
    (@Argument NVARCHAR(MAX))
RETURNS
    @List TABLE (Item NVARCHAR(20))
AS
BEGIN
    -- While a comma still exists in the argument
    WHILE (CHARINDEX(',', @Argument) > 0)
    BEGIN
        -- Add substring from start of argument to comma to result list (trim string to remove spaces)
        INSERT INTO @List
            VALUES (RTRIM(LTRIM(SUBSTRING(@Argument, 1, CHARINDEX(',', @Argument) - 1))))

        -- Remove the item from argument
        SET @Argument = SUBSTRING(@Argument, CHARINDEX(',', @Argument) + 1, LEN(@Argument))
    END

    -- Add last item to the list
    INSERT INTO @List VALUES (RTRIM(LTRIM(@Argument)))

    RETURN
END
GO

So if passed the argument ‘1, 2, 3, 4’, the function will return a table containing ‘1’, ‘2’, ‘3’ and ‘4’. The reason I wrote the function was because I had to wrap a set of queries in SQL stored procedures to provide an interface. The queries were pretty complex but for this example, I will only use a single table, let’s say a table containing employee information:

CREATE TABLE Employee
(
    ID INT PRIMARY KEY,
    Name NVARCHAR(200),
    Salary INT
)
GO

We need to wrap a query that returns the salary for a given list of employees, the list being provided as a comma-separated string of IDs. There are multiple ways to do this, I will just give a good idea and a bad idea.

Good Idea

CREATE PROCEDURE GetSalaries
    (@Employee_IDs NVARCHAR(MAX))
AS
    SELECT ID, Salary
        FROM Employee
        WHERE ID IN (SELECT CAST (Item AS INT) FROM Split(@Employee_IDs))
GO

Using the initial function, the input is split into individual IDs and cast as INT. ID and Salary are extracted from the table with the condition that ID is in the list. So for the sample data

INSERT INTO Employee VALUES (1, 'John Doe', 10000)
INSERT INTO Employee VALUES (2, 'Jane Doe', 20000)
INSERT INTO Employee VALUES (3, 'John Doe the 2nd', 30000)
GO

Calling

GetSalaries '1, 2, 4'

should return

1 10000
2 20000

(as ID 4 doesn’t exist in the table and ID 3 is not in the argument).

Bad Idea

CREATE PROCEDURE GetSalaries
    (@Employee_IDs NVARCHAR(MAX))
AS
    EXEC ('SELECT ID, Salary FROM Employee WHERE ID IN (' + @Employee_IDs + ')')
GO

The above procedure, using dynamic SQL, should behave similar to the previous procedure:

GetSalaries '1, 2, 4'

should still return

1 10000
2 20000

It’s even shorter. And it doesn’t require a Split function to parse the input. So why I call this a bad idea? The stored procedure will be provided as an interface, meaning the input will be beyond our control. And input could also be

GetSalaries '1); DROP TABLE Employee --'

Run it and see what happens😉

From → code complete

Leave a Comment

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: