Using dynamic sql in a procedure to insert data into schema tables

Question

I hope that you are all doing well.

I've been working on a project where I need to store data about my college that includes ID numbers, names, contact details etc.

I'm having a bit of difficulty in creating a stored procedure that will be able to insert data into a specified schema.table_name. The procedure must be able to allow the EXEC command to specify which schema you would like the insert data into. The table_name will stay the same for all the 14 schemas. The following code sample is what I have come up with but it doesn't seem to work:

CREATE PROCEDURE AddStudent_proc(@campus varchar(50), @StudentID numeric(4,0), @Name varchar(50), @Surname varchar(50), @ID_numeric numeric(13,0), @Address varchar(100))
AS
BEGIN
    DECLARE @dynamic varchar(MAX)
    SET @dynamic = 'INSERT INTO ['+quotename(@campus)+'].Student_tbl(
    StudentID,
    Name,
    Surname,
    ID_numeric,
    Address
    )
    VALUES('+quotename(@StudentID)+','+quotename(@Name)+','+quotename(@Surname)+','+quotename(@ID_numeric)+','+quotename(@Address)+');'
    EXEC (@dynamic);
END
GO

My entire structure can be found here

I'd appreciate any help on this topic as I am still quite new to SQL as a whole.

Thanks in advance.

Answer 1

You don't need to use quotename for data - as the name of the function implies, it should be used with names (A.K.A identifiers).
Also, when you are using quotename it addeds [ and ] around the value it receives, so no point of adding them again (['+quotename(@campus)+'] in your code).

I would recommend three improvements to the procedure you have now:

1. Change the data type of @campus to sysname - this is a special data type synonym to nvarchar(128) not null used by SQL Server for all identifiers.
2. white-list the schema name.
   This is a critical change to protect against SQL Injection attacks.
   Anything that can't be parameterized needs to be white-listed.
3. use sp_ExecuteSql instead of EXEC

This will result in a better stored procedure because it eliminates the threat of SQL Injection.

I've written a couple of blog posts that adds some more information and background on this subject: The do’s and don’ts of dynamic SQL for SQL Server and Back to basics: SQL Injection.

Anyway, here's how I would write this procedure:

CREATE PROCEDURE AddStudent_proc(
    @campus sysname, 
    @StudentID numeric(4,0), 
    @Name varchar(50), 
    @Surname varchar(50), 
    @ID_numeric numeric(13,0), 
    @Address varchar(100)
)
AS
BEGIN

    IF EXISTS(
        SELECT 1
        FROM Sys.Schemas
        WHERE name = @campus
    )
    BEGIN

    DECLARE @dynamic nvarchar(4000), 
            @paramDefinition nvarchar(4000)


        SELECT @dynamic = N'INSERT INTO '+ quotename(@campus) + N'.Student_tbl (
            StudentID,
            Name,
            Surname,
            ID_numeric,
            Address
        )
        VALUES(@StudentID, @Name, @Surname, @ID_numeric, @Address)',
        @paramDefinition = 
          N'@StudentID numeric(4,0), 
            @Name varchar(50), 
            @Surname varchar(50), 
            @ID_numeric numeric(13,0), 
            @Address varchar(100)'

        EXEC sp_executeSql @dynamic, @paramDefinition, @StudentID, @Name, @Surname, @ID_numeric, @Address;
    END
END
GO