CODEWITHSUNDEEP
sqlsql-server
user3057678
user3057678

Reputation: 315

Dynamic SQL Server stored procedure with table name as input param

I have the following stored procedure

CREATE PROCEDURE [dbo].[Insert]
    @Service varchar(max),
    @TableName varchar(100),
    @Query varchar(250),
    @Results varchar(max),
    @CreatedDate datetime,
    @ExpirationDate datetime
AS
BEGIN
    SET NOCOUNT ON;

    BEGIN TRANSACTION
        DECLARE @SQL NVARCHAR(MAX), @ParmDefinition NVARCHAR(MAX)
        DECLARE @q1 VARCHAR(MAX), @rez1 VARCHAR(MAX),  
                @date1 DATETIME, @date2 DATETIME

        DECLARE @tablename VARCHAR(MAX) = @Service + '.' + @TableName

        SET @SQL = N'if not EXISTS (select @q from ' + @tablename + ' where Query = @q) insert into ' + @tablename + ' values(@q, @rez, @date1, @date2)'

        SET @ParmDefinition = N'@q varchar(max), @rez varchar(max), 
                                @date1 datetime, @date2 datetime'

        EXECUTE sp_executeSQL      -- Dynamic T-SQL
            @SQL,
            @ParmDefinition,
            @q = @Query,
            @rez = @Results,
            @date1= @CreatedDate,
            @date2 = @ExpirationDate

        COMMIT TRANSACTION
END

When I try to execute it, it doesn't insert anything, 0 rows

If I execute the code without the stored procedure, like a single query it inserts

Am I missing something?

Upvotes: 2

Views: 5480

Answers (1)

M.Ali
M.Ali

Reputation: 69594

There are a lot of things you have done in your question which doesnt make any sense to me, Why do you need to declare all these variables inside your procedure.

Yes true you are using parametrised query to protect yourself against sql injection attack, yet you left a hole by concatenating the object names (Table and database name), yes you will need to concatenate them but you can use QUOTENAME() function around them passed parameters and enforce sql server to put square brackets around these parameters and force sql server to treat them as nothing else but object names.

And Selecting a variable in IF EXISTS not make much sense. Select 1 which returns true if a row is found with matching criteria , and if no row is found it will simply insert a row.

Only declare variables that needs to declared, otherwise this make it look like a mess and difficult to debug. As they say Keep it simple :)

Also use appropriate data types for your parameters, @Service I believe is your database name why does it need to be a VARCHAR(MAX) data type, use the data type specific to store Sql Server Object names SYSNAME. 

CREATE PROCEDURE [dbo].[Insert]
    @Service   SYSNAME,
    @TableName SYSNAME,
    @Query varchar(250),
    @Results varchar(max),
    @CreatedDate datetime,
    @ExpirationDate datetime
AS
BEGIN
 SET NOCOUNT ON;

 BEGIN TRANSACTION
    DECLARE @SQL NVARCHAR(MAX), @ParmDefinition NVARCHAR(MAX)

    SET @SQL = N'IF NOT EXISTS (select 1 from ' + QUOTENAME(@Service) + '.' + QUOTENAME(@TableName)
             + N' where Query = @q) ' 
             + N'insert into ' + QUOTENAME(@Service) + '.' + QUOTENAME(@TableName) 
             + N' values(@q, @rez, @date1, @date2)'

    SET @ParmDefinition = N'@q varchar(250), @rez varchar(max), 
                            @date1 datetime, @date2 datetime'

    EXECUTE sp_executeSQL @SQL
                         ,@ParmDefinition
                         ,@q = @Query
                         ,@rez = @Results
                         ,@date1= @CreatedDate
                         ,@date2 = @ExpirationDate

 COMMIT TRANSACTION
END

Upvotes: 3

Related Questions