ExecuteScalar vs ExecuteNonQuery when returning an identity value

Question

Trying to figure out if it's best to use ExecuteScalar or ExecuteNonQuery if I want to return the identity column of a newly inserted row. I have read this question and I understand the differences there, but when looking over some code I wrote a few weeks ago (whilst heavily borrowing from this site) I found that in my inserts I was using ExecuteScalar, like so:

public static int SaveTest(Test newTest)
{
    var conn = DbConnect.Connection();
    const string sqlString = "INSERT INTO dbo.Tests ( Tester , Premise ) " +
                             "               VALUES ( @tester , @premise ) " +
                             "SET @newId = SCOPE_IDENTITY(); ";
    using (conn)
    {
        using (var cmd = new SqlCommand(sqlString, conn))
        {
            cmd.Parameters.AddWithValue("@tester", newTest.tester);
            cmd.Parameters.AddWithValue("@premise", newTest.premise);
            cmd.Parameters.Add("@newId", SqlDbType.Int).Direction = ParameterDirection.Output;

            cmd.CommandType = CommandType.Text;
            conn.Open();
            cmd.ExecuteScalar();

            return (int) cmd.Parameters["@newId"].Value;
        }
    }
}

This works fine for what I need, so I'm wondering

1. Whether I should be using ExecuteNonQuery here because it is "more proper" for doing inserts?
2. Would retrieving the identity value be the same either way since I'm using an output parameter?
3. Are there any performance hits associated with one way or the other?
4. Is there generally a better way to do this overall?

I'm using Visual Studio 2010, .NET 4.0, and SQL Server 2008r2, in case that makes any difference.

Dailey

(1) why is ExecuteNonQuery "more proper"? (2) have you considered using stored procedures? If not, why not? It would definitely help clean up all of the ad hoc SQL you're putting into your app - which, when you have to change it, means you have to re-compile and re-deploy the app. – Vigen

Hmmm... ExecuteNonQuery is usually for executing SQL that does not expect to return a result. ExecuteScalar returns a value so you don't need to pass through parameters. You could change the last part of your SQL to SELECT SCOPE_IDENTITY(); then use return (int)cmd.ExecuteScalar(); – Overlap

I use ExecuteScalar because i use SELECT SCOPE_IDENTITY without an output parameter, hence retrieve a single value which is the purpose of ExecuteScalar. https://mcmap.net/q/152269/-return-value-from-sql-server-insert-command-using-c – Merrymerryandrew

Well an important thing to remember is your structure of your application, the goal, and the simplest method. As Aaron stated you could go the Stored Procedure route; you could even utilize large queries to pull data and compile them within your code. It is a matter of preference. But Sam is is correct for 'general' purpose. – Hanser

@AaronBertrand "More proper" because it's an insert, and the link I included (and others) said ExecuteNonQuery was for inserts. Also, the "quotes" were partly because I didn't necessarily believe it was more proper. Yes I did consider stored procedures and have used them in the past, though it didn't seem necessary here. Besides, changing the query would disrupt the other parts of the code enough that I'd have to redeploy anyway. – Dailey

@Techturtle I don't necessarily agree. A lot of query tuning does not actually affect the input or output of a query. – Vigen

@TimSchmelter and @Overlap I changed one of my queries to match what you suggested, but I'm getting System.InvalidCastException: Specified cast is not valid when I try to run it. It worked before with the output parameter... any ideas why it couldn't do the cast here? I also tried assigning it to an int variable before the return, but same result. – Dailey

@techturtle: Have a look at my linked answer (you have to cast it also in sql since it's a numeric) – Merrymerryandrew

@TimSchmelter that did it. I would have never guessed the whole numeric thing, esp since my identity column was an int (and it worked w/o casting on the output param). I even went and found the reason why. – Dailey

You can't cast to int, but can use Convert class in .Net Framework Convert.ToInt32/64 or Convert.ChangeType – Amur

Answer 1

As suggested by Aaron, a stored procedure would make it faster because it saves Sql Server the work of compiling your SQL batch. However, you could still go with either approach: ExecuteScalar or ExecuteNonQuery. IMHO, the performance difference between them is so small, that either method is just as "proper".

Having said that, I don't see the point of using ExecuteScalar if you are grabbing the identity value from an output parameter. In that case, the value returned by ExecuteScalar becomes useless.

An approach that I like because it requires less code, uses ExecuteScalar without output parameters:

public static int SaveTest(Test newTest)
{
    var conn = DbConnect.Connection();
    const string sqlString = "INSERT INTO dbo.Tests ( Tester , Premise ) " +
                             "               VALUES ( @tester , @premise ) " +
                             "SELECT SCOPE_IDENTITY()";
    using (conn)
    {
        using (var cmd = new SqlCommand(sqlString, conn))
        {
            cmd.Parameters.AddWithValue("@tester", newTest.tester);
            cmd.Parameters.AddWithValue("@premise", newTest.premise);

            cmd.CommandType = CommandType.Text;
            conn.Open();
            return (int) (decimal) cmd.ExecuteScalar();

        }
    }
}

Happy programming!

EDIT: Note that we need to cast twice: from object to decimal, and then to int (thanks to techturtle for noting this).

Snailfish

I ran into the casting problem as well. But sometimes it is not needed, wonder why? – Franks

I use this approach as well (ExecuteScalar with SCOPE_IDENTITY appended to the SQL command. The only "gotcha" is if you take this approach with a generic insert function and pass it a table that (gasp) doesn't have an identity driven primary key. If so you'll get an error Conversion from type 'DBNull' to type 'String' is not valid in VB.NET (ask me how I know) and I assume you'll get the c# equivalent of that error too. – Laith

thanks for the (decimal) part. It was needed!!! – Lengthen

Casting to Int32 would also work return (Int32) cmd.ExecuteScalar();. More about int/Int32 in this answer: stackoverflow.com/a/62738 – Sputum