|
In many cases a stored procedure would offer some benefits.
The real horror here is that the parameters were added with string formatting. Command parameters would have been a far better choice. This way, depending on where the parameters come from, the door is open for SQL injection attacks.
A while ago he asked me what he should have printed on my business cards. I said 'Wizard'.
I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.
|
|
|
|
|
I think in that case you can use a in code SQL, but using SQL parameters.
Then, in you SQL command you add the parameters.
Something like this:
string query = "select * from table where column = @value";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value", textBox1.Text);
This SQL statments are cached (so if you execute it in short time intervals the SQL plan will be computed) and aren't vunerable to SQL injection.
|
|
|
|
|
How would you add the parameter for a query like: SELECT * FROM Table WHERE ID IN (123,124,125); ?
Lists are a bit tougher to handle in a proper way...
"When did ignorance become a point of view" - Dilbert
|
|
|
|
|
Personally I wouldn't. I wouldn't use IN at all, I'd find a way to have a table on which to JOIN instead.
The statement you present is a symptom of a poorly implemented system.
|
|
|
|
|
There was a thread on here recently discussing this, comparing IN, EXISTS and JOIN. I can't find it now, but if I remember correctly there was a link on there to a blog from one of the SQL Server tech-heads that explained why and how the three are not interchangeable.
|
|
|
|
|
That might be an interesting read; I'll take a look to see if I can find it.
On the other hand, JOIN can do what IN and EXISTS can do, but IN and EXISTS can't do what JOIN does.
P.S. I just searched the general database forum back to May 1 and didn't find it.
modified on Thursday, August 5, 2010 12:02 AM
|
|
|
|
|
That's because it wasn't in the general database forum.
You'll find it here.[^]
"When did ignorance become a point of view" - Dilbert
|
|
|
|
|
|
Using an SQL IN-clause is definetely not a design flaw. Forcing everything into JOIN's is on the other hand an odd self-imposed hinderence.
|
|
|
|
|
While it is true that one should "use the right tool for the right job", I have never used EXISTS (I have an Oracle background), and I have not used IN/NOT IN for many years, and never with SQL Server.
JOIN tends to scale better -- you may have an IN, EXISTS, or even a BETWEEN that has to be converted to a JOIN as the project becomes more complex; using a JOIN to begin with eases such maintenance.
JOIN allows you to configure a system by maintaining a table rather than modifying the code.
As with Jörgen's post, an IN with hard-coded values I especially discourage; they reek of the "magic numbers" code smell. A subquery on some table would at least improve maintainability.
|
|
|
|
|
PIEBALDconsult wrote: As with Jörgen's post, an IN with hard-coded values I especially discourage; they reek of the "magic numbers" code smell. A subquery on some table would at least improve maintainability.
I think I might have been unclear in my post. I never said they were hardcoded, I only wondered if he knew an easy way to add a list as a parameter and gave an example with using a list. Assume that that this list is dynamic and comes from the application.
Normally I would add that list to a temporary table and make a subquery or a join on that table. But it would be nice to be able to add that list as a parameter.
"When did ignorance become a point of view" - Dilbert
|
|
|
|
|
Jörgen Andersson wrote: list as a parameter
Indeed, you have me wondering whether or not a DataTable may be passed as a parameter. Though I'm sure that if so, that only SQL Server would support it, so it wouldn't be a general solution.
P.S. This[^] looks interesting.
P.P.S. And this[^].
modified on Friday, July 9, 2010 5:21 PM
|
|
|
|
|
Well, it seems that you can pass an array as a parameter to a stored procedure in both SQL serever[^] and Oracle[^]
"When did ignorance become a point of view" - Dilbert
|
|
|
|
|
Eureka!
In SQL Server 2008 (Express):
I created an Account table with ID (int) and Name (nvarchar) fields.
I populated the Account table with some records.
I created an IDdef User Defined Table Type with an ID (int) field.
In C# I instantiated a DataTable, added an ID (int) column.
Added some rows to the DataTable.
Then set up the following (db is an instance of one of my DALs, dt is the DataTable):
db.Command.CommandText = "SELECT * FROM Account WHERE ID IN ( SELECT ID FROM @IDs )" ;
System.Data.SqlClient.SqlParameter p =
new System.Data.SqlClient.SqlParameter
( "@IDs" , System.Data.SqlDbType.Structured ) ;
p.TypeName = "dbo.IDdef" ;
p.Value = dt ;
db.Command.Parameters.Add ( p ) ;
db.Open() ;
System.Data.IDataReader dr = db.Command.ExecuteReader
( System.Data.CommandBehavior.CloseConnection ) ;
And it works!
I then changed the statement to SELECT * FROM Account INNER JOIN @IDs IDs ON Account.ID=IDs.ID
and that works too!
|
|
|
|
|
My wife is going to kill me, I'm supposed to put up new wallpaper in the livingroom tomorrow. Not sit in front of the computer again.
"When did ignorance become a point of view" - Dilbert
|
|
|
|
|
It'll keep.
The wallpaper, that is.
|
|
|
|
|
I finally remembered to try SELECT * FROM Account WHERE EXISTS ( SELECT ID FROM @Param0 IDs WHERE IDs.ID=Account.ID ) , it works.
It appears that one of the differences between IN/EXISTS and JOIN is that the JOIN sorts (or maybe the others do).
|
|
|
|
|
PIEBALDconsult wrote: Personally I wouldn't. I wouldn't use IN at all, I'd find a way to have a table on which to JOIN instead.
A JOIN is a poor substitute for a NOT IN or NOT EXISTS . And they don't handle nulls the same way.
PIEBALDconsult wrote: The statement you present is a symptom of a poorly implemented system.
Agreed, but sometimes that's what you have.
"When did ignorance become a point of view" - Dilbert
|
|
|
|
|
But your example uses IN, not NOT IN.
|
|
|
|
|
I would imagine this would work:
string query = "select * from table where ID in (@value1, @value2, @value3)";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value1", 123);
cmd.Parameters.Add("@value2", 124);
cmd.Parameters.Add("@value3", 125);
If not, this surely would:
string query = "select * from table where (ID = @value1 or ID = @value2 or ID = @value3)";
SqlCommand cmd = new SqlCommand(query);
cmd.Parameters.Add("@value1", 123);
cmd.Parameters.Add("@value2", 124);
cmd.Parameters.Add("@value3", 125);
|
|
|
|
|
It does as long as your list isn't dynamic, but assume your list has a varying number of values...
"When did ignorance become a point of view" - Dilbert
|
|
|
|
|
That is true, things get a bit problematic when you have many or a variable amount of parameters.
First, I would simply try to avoid the situation if that is possible. Perhaps it is possible to replace the list with a nested select statement. It really depends on the case at hand.
If that's not possible, I would put together the SQL command in my code. With a variable amount of parameters, I would simply use a loop to add the parameter placeholders to the SQL string and the parameters themselves to the command object. Taken to the extreme, this can become very ugly and results in absolutely unmaintainable code. In my current project here at work some genius tried to replace the complete data access layer with one single function which constructs every needed SQL statement in a few thousand (!) lines of spaghetti code. But, like with everything, there is no problem as long as it is used sparingly and with at least some thought.
Edit: Here is the beginning of that monster function, a worthy candidate for this board as well:
public static DataSet ArtikelHelpListe(int auswahl, string artnr, string artbez, string lnam,
string lnr, string lartnr, string wg, string mkenz,
string ean, string gab, bool ges, bool bug, bool bau,
bool eks, int stanort, string uid)
{
....
}
A while ago he asked me what he should have printed on my business cards. I said 'Wizard'.
I read books which nobody else understand. Then I do something which nobody understands. After that the computer does something which nobody understands. When asked, I say things about the results which nobody understand. But everybody expects miracles from me on a regular basis. Looks to me like the classical definition of a wizard.
|
|
|
|
|
A for(each) loop to create the parameters and another to load the values works pretty well. Assuming the list of values is in some form of collection.
|
|
|
|
|
It isn't; pay no attention to the stored procedure fan boys.
|
|
|
|
|
While we are at it, could someone please explain to me why inline SQL might be more vulnerable to SQL injection than a stored proc. Having thought about it all afternoon, i must admit that i just can't see it. Am i blind?
|
|
|
|