2

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?

5
  • are you sure that using @TableName and @tablename as 2 different variables works ? Commented Oct 10, 2014 at 11:52
  • Try using PRINT and see whats happening during execution.The passing of parameters to execution may be issue.Please provide some sample for the parameters Commented Oct 10, 2014 at 11:53
  • modified with another variable @dynamictable,still i dont have inserts Commented Oct 10, 2014 at 11:53
  • print gives this: if not EXISTS (select Query from dbo.TestTab where Query = @q) insert into dbo.TestTab values(@q,@rez,@date1,@date2) Commented Oct 10, 2014 at 11:56
  • I think a better question would be why do you have so many tables in the same format that you require a generic insert procedure? Why not have a single table with a column that distinguishes the types of records, or have a separate insert procedure for each table? Commented Oct 10, 2014 at 12:42

1 Answer 1

3

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
Sign up to request clarification or add additional context in comments.

1 Comment

works great. thank you for all the help, and next time i will minimize using variables like that,

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.