C# SQL insert command

Question

Can anyone tell me the following 2 ways of inserting record creates better performance?

Case 1

SqlCommand cmd = new SqlCommand();

for (int i = 0; i < 10000; i++)
{
  cmd = new SqlCommand("insert into test(id, name) value('" + i + "', '" + i + "')");
  cmd.ExecuteNonQuery();
}

Case 2

string sql = null;

for (int i = 0; i < 10000; i++)
{
  sql += "insert into test(id, name) value('" + i + "', '" + i + "')";
}

SqlCommand cmd = new SqlCommand(sql, conn);
cmd.ExecuteNonQuery();

Groomsman

Have you run a simple timing test? Execute each one 10,000 times, and see which one runs faster. – Dioptrics

Same, but case one is preferred. Same because you'll use connection pooling. – Soppy

First of all: STOP concatenating together your SQL code!! This is an invitation to hacker to attack you with SQL injection! Use parametrized queries instead! – Myogenic

@DanAndrews: really?!?!?? You think creating 10'000 SqlCommand instances and executing them one by one is just as fast as creating a single instance and executing it just once??? – Myogenic

You are right, it would be faster. – Soppy

Answer 1

First of all: STOP concatenating together your SQL code!! This is an invitation to hackers everywhere to attack you with SQL injection! Use parametrized queries instead!

I would use this solution: create a single SqlCommand with a parametrized query, and execute that:

string stmt = "INSERT INTO dbo.Test(id, name) VALUES(@ID, @Name)";

SqlCommand cmd = new SqlCommand(smt, _connection);
cmd.Parameters.Add("@ID", SqlDbType.Int);
cmd.Parameters.Add("@Name", SqlDbType.VarChar, 100);

for (int i = 0; i < 10000; i++)
{
    cmd.Parameters["@ID"].Value = i;
    cmd.Parameters["@Name"].Value = i.ToString();

    cmd.ExecuteNonQuery();
}

or use SqlBulkCopy, especially if you're inserting even more than 10'000 rows.

Myogenic

While I usually agree that passing in unsanitized parameters directly to concatenated SQL is bad - in the context of this code it is literally harmless. There are no User Input values to be concerned with - it looks like a quick one off operation. – Matri

The new recommended way is to use AddWithValue – Cleptomania

@Si8: MOST DEFINITELY NOT!!! - see Dan Guzman says: "AddWithValue is Evil" - please read the article and stop using it! – Myogenic

Hmmmmm i remember seeing it somewhere but now can't find it. And you are right... Thanks. – Cleptomania

@Si8: I'd be interested to know who or what site is propagating this ...... really really bad advice....... :-) – Myogenic

I've always used Add but recently switched to AddWithValue, because of it. Good thing I came across your advice and note. – Cleptomania

@Myogenic Now i know where, it says it in the VS Dev application...It says .Add() is obsolete and to use .AddWithValue(); – Cleptomania

@Si8: scary! That's official MS docs..... but yes - that one overload has been deprecated - but you should definitely NOT replace it with AddWithValue ! Thanks – Myogenic

I would imagine VS and C# being Microsoft, they wouldn't recommend something not accurate but again having used SharePoint that is definitely not the case! I will stick with Add() even if VS gives me warning. – Cleptomania

@Si8: just use this overload: Add(String, SqlDbType) and explicitly define the desired datatype - set the actual value of the parameter by using the .Value = xxx syntax - as shown in my response – Myogenic

Answer 2

The second approach looks faster than #1 because you send the INSERT commands at once. In the first there's a round trip to the SQL server for each ExecuteNonQuery.

But you should try the bulk insert command: BULK INSERT (Transact-SQL), I guess you'll get a better performance than any one of the options you provided.

[]'s

Cornelia

While this might be a good idea, how does it answer the question of "which is faster, option 1 or option 2?". You've said "Do option 3 instead". – Marquesan

@KenWhite I think the answer is plenty appropriate. It doesn't directly answer the question because the OP is using an inefficient process. I have a hunch the OP is not in search of exactly which way uses fewer CPU cycles (as that they could easily test themselves if that was exactly what they wanted to know). Instead, I believe the OP is trying to understand which way is more efficient, and Fabio has provided a solution better than either of the OP's. – Lowndes

It doesn't. As you've said, it's a third option. Sometimes we don't get the exactly answer we want, but we got somwthing that make us look at things from a different point of view. []'s – Cornelia

I really hate to see answers upvoted when they don't answer a very specific question that was asked. – Soppy

If it's a third option, it's fine to add as a footnote to the answer that actually answers the question asked, or as a comment to the original question. It's not an answer to the question asked, and shouldn't be posted as such. I could have just downvoted or flagged it as "not an answer"; I didn't. I gave the poster a chance to either expand on the answer provided or delete it first. Answers should actually do that - answer the question asked. Additional advice or information on alternatives can then be provided. – Marquesan

@KenWhite I suppose it's worth mentioning that while marc_s provided an answer that doesn't even come close to "answering the specific question asked" (it is definitely a 'do option 3 instead' answer), it's unquestionably the best answer. Out of curiosity, do you feel the same way about his answer as you do about Fabio's? – Lowndes

Yep, I do. :) He posted after the discussion in this question started However, his post was also more than a single line of text with a URL (which in itself is also not considered a proper answer, BTW, especially when that URL is to an off-site location, even if that site is MS). @Fabio, much better. :) Nicely done. – Marquesan

Answer 3

I don't think the second one will work.

There is however a syntax in SQL Server 2008 for inserting multiple rows in a single INSERT statement and I think that will be faster than both the options you proposed:

INSERT INTO test (id, name)
VALUES
('1', 'foo'),
('2', 'bar'),
('3', 'baz')
 -- etc...

However if you really want high performance, consider using the SqlBulkCopy class.

Eslinger

Answer 4

It should be noted exactly that as-is, neither case will work.

Case #1 requires a connection to be specified.

Case #2 requires you to end your statements with a semi-colon in order to run multiple commands, like so:

string sql = null;

for (int i = 0; i < 10000; i++)
{
  sql += "insert into test(id, name) value('" + i + "', '" + i + "');";
}

SqlCommand cmd = new SqlCommand(sql, conn);
cmd.ExecuteNonQuery();

Ultimately the best way would be for you to just test it yourself on several thousand rows. My guess would be that the Case #2 would be better for performance because not only would it require setting up only a single SqlCommand object, but it only hits the database a single time.

Lowndes

Answer 5

The second one is probably faster, because you end up with a single roundtrip. Both are equally awful, because you do not use parameterized queries.

Winker

@Kiley: I can't see a point of you posting this here, other than inciting an argument. – Winker