1

I have an SQL Server stored procedure which do an INSERT INTO.

I'm trying to get in C# the ID of the inserted row.

I can't debug so I don't know where is the error...

The row is perfectly add in the database but it always catch an exception and the ID is not returned

STORED PROCEDURE

USE [DatabaseAzerty]
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER PROCEDURE [dbo].[addLog] 
    -- Add the parameters for the stored procedure here
    @table varchar(max), 
    @date date,
    @version varchar(max),
    @process varchar(max),
    @level varchar(max),
    @message varchar(max),
    @stacktrace varchar(max),
    @user varchar(max),
    @environmentID varchar(max),
    @UUID varchar(max),
    @UDID varchar(max),
    @transactionID int,
    @new_identity int OUTPUT

AS
BEGIN
    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;

    -- Insert statements for procedure here
 --   DECLARE @sql NVARCHAR(MAX);

 --   SET @sql = 'INSERT INTO dbo.' + @table + '([Timestamp], [Version],Process,[Level],[Message],StackTrace,[User],EnvironmentID,UUID,UDID,TransactionID) VALUES (' + CAST(@date AS VARCHAR(MAX)) + ',' + @version + ',' + @process + ','+ @level + ','+ @message + ','+ @stacktrace + ','+ @user + ','+ @environmentID + ','+ @UUID + ','+ @UDID + ',' + @transactionID + ')'
    --EXEC @sql
    DECLARE @SQL NVARCHAR(MAX);

SET @SQL =  ' INSERT INTO dbo.' + @Table;
SET @SQL += '   ([Timestamp], [Version],Process,[Level],[Message],StackTrace,[User],EnvironmentID,UUID,UDID,TransactionID)';
SET @SQL += ' VALUES';
SET @SQL += '   (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10);';

EXECUTE sp_executesql
      @SQL
    , N'@p0 DATE, @p1 VARCHAR(MAX), @p2 VARCHAR(MAX), @p3 VARCHAR(MAX), @p4 VARCHAR(MAX), @p5 VARCHAR(MAX), @p6 VARCHAR(MAX), @p7 VARCHAR(MAX), @p8 VARCHAR(MAX), @p9 VARCHAR(MAX), @p10 int'
    , @p0 = @date
    , @p1 = @version
    , @p2 = @process
    , @p3 = @level
    , @p4 = @message
    , @p5 = @stacktrace
    , @p6 = @user
    , @p7 = @environmentID
    , @p8 = @UUID
    , @p9 = @UDID
    , @p10 = @transactionID;

    SELECT @new_identity =  CAST(SCOPE_IDENTITY() AS INT);
END

C#

 SqlConnection conn = new SqlConnection(connectionString);


            var cmd = new SqlCommand("addLog", conn)
            {
                CommandType = System.Data.CommandType.StoredProcedure
            };

            cmd.Parameters.Add(new SqlParameter("@message", message));

            SqlParameter id = new SqlParameter("@new_identity", SqlDbType.Int);
            id.Direction = ParameterDirection.Output;
            cmd.Parameters.Add(id);

            conn.Open();
            cmd.ExecuteNonQuery();
            int returnedID = Convert.ToInt32(cmd.Parameters["@new_identity"].Value); 
            conn.Close();

            return returnedID;
Weedoze
  • 13,683
  • 1
  • 33
  • 63
  • can you show the complete addLog sp ? –  May 04 '15 at 12:30
  • All the procedure added – Weedoze May 04 '15 at 12:32
  • 1
    Concatenating statements is a very bad idea, whether you do it in C# or in T-SQL. You expose yourself to SQL injection attacks, failures just because someone typed a `'` and the code is very hard to maintain. You may have a type in the concatenated statements that you won't be able to find without actually profiling the resulting statements – Panagiotis Kanavos May 04 '15 at 12:33
  • Although in this case, you don't seem to pass any of the required parameters. Is the C# code complete or did you remove the parameter assignments? – Panagiotis Kanavos May 04 '15 at 12:34
  • I used this procedure after asking for help and this solution worked for me. I remove all the parameted assignements, sorry for not mentionning – Weedoze May 04 '15 at 12:36
  • try this link for more details http://www.aspsnippets.com/Articles/Getting-ID-of-the-newly-inserted-record-in-SQL-Server-Database-using-ADO.Net.aspx –  May 04 '15 at 12:38
  • Please show the exception!! .Also @new_identity will always be null, because you inserted the row on another scope, the sp_executesql scope. To make it to work you need to read the new identity inside the sql passed to sp_executesql and fill an output parameter there. – Jesús López May 04 '15 at 12:38
  • @Weedoze this is very error prone code. You just found out how easy it is to forget a single statement and never see it. A better solution is to use a simple INSERT statement with an OUTPUT clause to return the new ID as [shown here](http://stackoverflow.com/questions/10999396/how-do-i-use-an-insert-statements-output-clause-to-get-the-identity-value) – Panagiotis Kanavos May 04 '15 at 12:39
  • @PanagiotisKanavos The problem is that I have the name of my table in the parameters – Weedoze May 04 '15 at 12:40
  • @Weedoze don't. As I said, this is a very *very* bad idea. You gain nothing by this stored procedure (certainly not reusability) but you do 'gain" a lot of security and performance problems. A single `INSERT` is enough both to add log entries and return their IDs. – Panagiotis Kanavos May 04 '15 at 12:43
  • @Weedoze. If you use EntityLite, you would not need server side dynamic sql. No SQL Injection, this use case could be implemented with EntityLite – Jesús López May 04 '15 at 12:43
  • I was asked to use a stored procedure and then call it in c#. I have to use this way .... – Weedoze May 04 '15 at 12:45
  • Ok, then please validate @table parameter against catalog views. – Jesús López May 04 '15 at 12:46
  • @Weedoze how many tables are you going to target? If you only have to target a few tables, you can create one stored procedure for each. I doubt anyone asked you to deliberately harm performance and open security holes – Panagiotis Kanavos May 04 '15 at 13:08
  • There is not a fixed number of table... I will see if I can use something else. By the way M.Ali provided the solution. Thx for your time ! – Weedoze May 04 '15 at 13:10

3 Answers3

3

You need to pass the output parameter to your dynamic sql.

ALTER PROCEDURE [dbo].[addLog] 
    -- Add the parameters for the stored procedure here
    @table          SYSNAME,  --<-- use SYSNAME data type for Sql Server objects
    @date           date,
    @version        varchar(max),
    @process        varchar(max),
    @level          varchar(max),
    @message        varchar(max),
    @stacktrace     varchar(max),
    @user           varchar(max),
    @environmentID  varchar(max),
    @UUID           varchar(max),
    @UDID           varchar(max),
    @transactionID  int,
    @new_identity   int OUTPUT

AS
BEGIN
    SET NOCOUNT ON;
    DECLARE @SQL NVARCHAR(MAX);

SET @SQL = N' INSERT INTO dbo.' + QUOTENAME(@Table)       --<-- use QUOTENAME function here (Sql Injection protection)
         + N'   ([Timestamp], [Version],Process,[Level],[Message]
                       ,StackTrace,[User],EnvironmentID,UUID,UDID,TransactionID)'
         + N' VALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10) '
         + N' SELECT @new_identity =  SCOPE_IDENTITY(); ' --<-- Capture Identity value here

EXECUTE sp_executesql
      @SQL
    , N'@p0 DATE, @p1 VARCHAR(MAX), @p2 VARCHAR(MAX), @p3 VARCHAR(MAX)
        , @p4 VARCHAR(MAX), @p5 VARCHAR(MAX), @p6 VARCHAR(MAX), @p7 VARCHAR(MAX)
        , @p8 VARCHAR(MAX), @p9 VARCHAR(MAX), @p10 int, @new_identity INT OUTPUT'
    , @p0 = @date
    , @p1 = @version
    , @p2 = @process
    , @p3 = @level
    , @p4 = @message
    , @p5 = @stacktrace
    , @p6 = @user
    , @p7 = @environmentID
    , @p8 = @UUID
    , @p9 = @UDID
    , @p10 = @transactionID
    , @new_identity = @new_identity OUTPUT

END
M.Ali
  • 67,945
  • 13
  • 101
  • 127
-1

use the try-finally:

int returnedID = 0;
//.....
try
{
            cmd.ExecuteNonQuery();
}
finally
{
            int returnedID = Convert.ToInt32(cmd.Parameters["@new_identity"].Value); 
}
Vladimir Semashkin
  • 1,270
  • 1
  • 10
  • 21
-1

Make Sure That Parameters @new_identity In SQL Declare OutPut. Close Connection before set returnedID Consider Using @@IDENTITY