Generic Insert stored proc : Runtime error

Question

The following code generates the primaey key for the new record to be inserted and inserts the record into a table, whose name and the values to be inserted are given as parameters to the stored procedure. I am getting a runtime error. I am using Visual Studio 2005 to work with SQL Server 2005 Express Edition

ALTER PROCEDURE spGenericInsert

(
    @insValueStr nvarchar(300), 
    @tblName nvarchar(10) 
)


AS

DECLARE @sql nvarchar(400)
DECLARE @params nvarchar(200)
DECLARE @insPrimaryKey nvarchar(10)
DECLARE @rowCountVal integer
DECLARE @prefix nvarchar(5)

--following gets the rowcount of the table--
SELECT @rowCountVal = ISNULL(SUM(spart.rows), 0)
 FROM sys.partitions spart 
 WHERE spart.object_id = object_id(@tblName) AND spart.index_id < 2

SET @rowCountVal = @rowCountVal+1




--Following Creates the Primary Key--
IF @tblName = 'DEFECT_LOG' 
 SET @prefix='DEF_'
ELSE IF @tblName='INV_Allocation_DB'
 SET @prefix='INV_'
ELSE IF @tblName='REQ_Master_DB'
 SET @prefix='REQ_'
ELSE IF @tblName='SW_Master_DB'
 SET @prefix='SWI_'
ELSE IF @tblName='HW_Master_DB'
 SET @prefix='HWI_' 


SET @insPrimaryKey= @prefix + RIGHT(replicate('0',5)+ convert(varchar(5),@rowCountVal),5) -- returns somethin like 'DEF_00005'


-- Following is for inserting into the table --

SELECT @sql =  N' INSERT INTO @tableName VALUES ' +
  N' ( @PrimaryKey , @ValueStr )'

SELECT @params = N'@tableName nvarchar(10), ' +
                    N'@PrimaryKey nvarchar(10), ' +
                    N'@ValueStr  nvarchar(300)'

EXEC sp_executesql @sql, @params, @tableName=@tblName, @PrimaryKey=@insPrimaryKey, @ValueStr=@insValueStr

Output Message:

Running [dbo].[spGenericInsert] ( @insValueStr = 2,"Hi",1/1/1987, @tblName = DEFECT_LOG ).

Must declare the table variable "@tableName".

No rows affected.

(0 row(s) returned)

@RETURN_VALUE = 0

Finished running [dbo].[spGenericInsert].

Answer 1

You are going to have to concatenate the table name directly into the string, as this cannot be parameterized:

SELECT @sql =   N' INSERT INTO [' + @tblName + '] VALUES ' +
            N' ( @PrimaryKey , @ValueStr )'

SELECT @params = N'@PrimaryKey nvarchar(10), ' +
                N'@ValueStr  nvarchar(300)'

To prevent injection attacks, you should white-list this table name. This also isn't robust if the table has other non-nullable columns, etc.

note: Personally, though, I don't think this is a good use of TSQL; it might be more appropriate to construct the command in the client (C# or whatever), and execute it as a parameterized command. There are use-cases for dynamic-SQL, but I'm not sure this is a good example of one.

Better yet, use your preferred ORM tool (LINQ-to-SQL, NHibernate, LLBLGen, Entity Framework, etc) to do all this for you, and concentrate on your actual problem domain.

Answer 2

what is white-listing a table name?

Answer 3

White list essentially means make sure that the table being passed in is a valid table that you want them to be able to insert into. Let's just say for arguments sake that table name is user provided, the user could then start inserting records into system tables.

You can do a white list check by bouncing the table name of the sysobjects table:

select * from sysobjects where name=@tblname and xType='U'

However as Marc suggested this is not a good use of TSQL, and your better off handling this in the app tier as a paramatized query.

Answer 4

Agree with Marc- overall this is an extremely poor idea. Generic inserts/updates or deletes cause problems for the database eventually.

Another point is that this process will have problems when two users run simulutaneously against the same table as they will try to insert the same Primary Key.