1

I have a sqlhelper class that includes an overloaded ExecuteNonQuery: one with just one parameter (commandText) and another one with two parameters (commandText, SqlParameter[]).

Assuming I have a stand-alone console application with no user interaction, and I will call a stored procedure that will just update a table with 3 parameters, what are the benefits of using SqlParameter[] if I can just as easily build the string and just send it as commandText?

In other words, why use the following:

SqlParameter[] parameters =
    {    
        new SqlParameter("parm1" SqlDbType.VarChar, 3),
        new SqlParameter("parm2", SqlDbType.VarChar, 8),
        new SqlParameter("parm3", SqlDbType.VarChar, 2),
        new SqlParameter("parm4", SqlDbType.VarChar, 4)
    };

parameters[0].Value = p1;
parameters[1].Value = p2;
parameters[2].Value = p3;
parameters[3].Value = p4;

When I can use something like this:

strQueryToRun = string.Format("exec updateTable {0}, {1}, {2}, {3}", p1, p2, p3, p4);

This is a stand-alone console application so there's no possibility of sql injection.

Thanks.

Improve this question
5
  • 8
    To avoid SQL injection. Commented Feb 25, 2014 at 20:54
  • You talk about a particular case. But what if CommandText is a simple Sql like SELECT * from T1 where A=@a1 AND b=@b1 Commented Feb 25, 2014 at 20:58
  • You may see: stackoverflow.com/questions/12777667/… that how SQL injection is possible with stored procedures Commented Feb 25, 2014 at 21:02
  • I updated the question. It's a stand-alone console application, so there's no possibility of sql injection. I forgot to mention this. Commented Feb 25, 2014 at 21:06
  • 2
    @user2520528: Your data is coming from somewhere, and likely at some point it's information the user is touching. Yes, in your case injection is (perhaps extremely) unlikely, but parameterizing is a best practice and the increase in complexity is generally somewhere between trivial and nominal, so why not? Especially when there are still other reasons to do it (see the answers). Commented Feb 25, 2014 at 21:07

3 Answers 3

Reset to default
6

The first and absolutely most important reason is so that your query does what you expect it to do, not what someone maliciously makes it do. Have a look at the Wikipedia article on SQL Injection.

In addition to mitigating (effectively eliminating) the risk of SQL injection, using parameters also allows SQL Server to take advantage of cached query plans. This is less of an issue in your specific instance (where you're simply calling a stored procedure, whose plan is almost certainly already compiled and cached), but this is a more general reason why you need to parameterize your queries.

Another reason (as pointed out by Ali in another answer) is that using the string.Format method is going to give your parameters whatever the string representation is of the native .NET type. For numerics, this is not an issue. For string types, you would have to enclose in single quotes and properly escape any embedded quotes (and likely other sanitizing routines). Using the parameter lets the SQL client libraries worry about how the data gets passed to the server.

That said, I would not use the code as you have written it above. I wouldn't construct an array of SqlParameters at all. There are a variety of ways to add a parameter to a SqlCommand (or DbCommand or whatever you're using), such as AddWithValue that provides a less verbose mechanism that is sufficient for most parameters that get added.

Even ignoring AddWithValue, I would still create individual variables for each parameter and name them something meaningful.

var parm1 = new SqlParameter("parm1", SqlDbType.VarChar, 3);
var parm2 = new SqlParameter("parm2", SqlDbType.VarChar, 8);
var parm3 = new SqlParameter("parm3", SqlDbType.VarChar, 2);
var parm4 = new SqlParameter("parm4", SqlDbType.VarChar, 4);

parm1.Value = p1;
parm2.Value = p2;
parm3.Value = p3;
parm4.Value = p4;

(Obviously a name like parm1 or parm2 is far from meaningful, but I would assume that your actual parameter names have more meaning than your example)

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Being best practice, I used SqlParameter, but I've noticed there are disadvantages to this. First, debugging is not straightforward: I can't see the actual stored proc call through VS debugging. It doesn't show up in SQL Profiler if the call breaks, so I can't get the call from there. Also, the SP accepts optional parameters, but SqlParameter doesn't allow a null value as possible parameter without DBNull.Value. So, considering that there won't be any sql injection and all parameters are numeric, what is the benefit of the SqlParameter? It seems it's more hassle than what it's worth.
@user2520528: This has already been answered a couple of times. If the parameter is truly optional, then it doesn't even need to be specified at all. If you specify it--whether through a parameter or not--then the optional value won't be used. Using parameters also allows you to be ignorant of how you would need to format the string literal for a given value.
As for not being able to see the stored proc call, I'm not really sure what you mean. The "actual" call is done through the TDS and parameters are not simply serialized into SQL (since that's the point). What you see in profiler is essentially a reconstruction of the statement and its parameters that you could run in SSMS. You can freely inspect any parameter value, and you can see the name of the procedure. What is missing?
Thanks for the post. Regarding the debugging, this is what I mean: My stored proc usp_WriteStuff has 3 parameters, and the last 2 are optional. In my C# code the 1st SqlParameter is set with a string and the last two are null. If I run "usp_WriteStuff 'test', null, null" in query analyzer it's fine. In my console app, it breaks with the error "The parameterized query expects the parameter '@LastName', which was not supplied". But, as far as I know, I have no idea what sql server is executing. It was after googling that I realized that SqlParameter needs to be set to DBNull.Value.
@user2520528: If you call @usp_WriteStuff 'test', null, null, you are not using the default values for the second and third parameter, you're explicitly supplying a null value. You would have to call usp_WriteStuff 'test' to use the default (i.e. what you're referring to as optional) values. And yes, you must explicitly use DBNull.Value in ADO.NET objects in order to represent a null database value. If you truly want to utilize the default value for that parameter, do not add the parameter to the command at all.
4

Simply because of two reasons:

  1. Parameters help to enforce strong data typing, which you lose using the string way!
  2. To avoid SQL injection

that's all.

Improve this answer

3 Comments

"To avoid SQL Injection". "That's all." That's a pretty big 'all'.
+1. Great point on the first reason (though I'd say #2 is more important)
@GeorgeStocker Yes! That's realy BIG.
1

To avoid SQL injection. For example if I make my arguments be 1, 2, 3, 4;\nDROP PROCEDURE updateTable; it would cause the following to be executed by your sever.

exec updateTable 1, 2, 3, 4;
DROP PROCEDURE updateTable;
Improve this answer

2 Comments

Even though sql injection is not my case, why would SqlParameters help in this example? In this case, the 4th SqlParameter would equal "4;\nDROP PROCEDURE updateTable" and the stored procedure would be dropped.
I was referring to your string.Format("exec updateTable {0}, {1}, {2}, {3}", p1, p2, p3, p4); example. In that example passing in 4;\nDROP PROCEDURE; will be executing a procedure with 4 as the 4th parameter and you will also be executing the sql command DROP PROCEDURE updateTable;. You are correct that if you did use SqlParameters the 4th parameter would be 4;\nDROP PROCEDURE updateTable and the command would fail. That is the point I was trying to make.

Your Answer

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