String.Format???
-
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.
-
I agree, that might be the case, but the fact still remains (as CDP1802 noted), that having SQL statement in code formatted with parameters that might come from e.g. UI text-boxes, represents great vulnerability to SQL injection attacks. Otherwise I understand that sometimes there is no other way, nevertheless in-code SQL can be used wisely or not.
Did you look at the code in the OP? You can't do SQL injection if your parameter data is strongly typed Int32 and DateTime values. If it was a string that's a different story, but if you are doing extra code to make sure no one is slipping SQL keywords into your ints then you are wasting a lot of time way overarchitecting.
-
Admittedly a SP would be much a nicer solution, but in real life you may have to work with databases where you are nowhere near getting authorized to implement a SP. Think of implementing a reporting system for at large financial institution as a consultant. What do you think their reply would be if you came saying "I need a dozen new stored procedures in your central DB2-database"? The polite answers would be something along the lines of "I'm sorry but that won't be possible", "Are you quite sure this is needed?" etc, etc. The impolite answer would be to find someone else to do the job.
Søren Turin wrote:
a nicer solution
Not for a simple SELECT.
-
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
-
Personally I wouldn't. I wouldn't use
INat all, I'd find a way to have a table on which toJOINinstead. The statement you present is a symptom of a poorly implemented system.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
JOINis a poor substitute for aNOT INorNOT 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. :sigh:
"When did ignorance become a point of view" - Dilbert
-
Ah, good, thanks.
-
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
JOINis a poor substitute for aNOT INorNOT 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. :sigh:
"When did ignorance become a point of view" - Dilbert
But your example uses IN, not NOT IN. :-D
-
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
-
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. :sigh: P.S. This[^] looks interesting. P.P.S. And this[^].
modified on Friday, July 9, 2010 5:21 PM
-
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. :sigh: 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
-
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
-
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! :-D I then changed the statement to
SELECT * FROM Account INNER JOIN @IDs IDs ON Account.ID=IDs.IDand that works too! :jig: :jig: :jig: :jig: -
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! :-D I then changed the statement to
SELECT * FROM Account INNER JOIN @IDs IDs ON Account.ID=IDs.IDand that works too! :jig: :jig: :jig: :jig: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
-
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. :-D
-
Did you look at the code in the OP? You can't do SQL injection if your parameter data is strongly typed Int32 and DateTime values. If it was a string that's a different story, but if you are doing extra code to make sure no one is slipping SQL keywords into your ints then you are wasting a lot of time way overarchitecting.
I was making general observation about in-code SQL. I believe there are some valid causes for using in-code SQL, but it is generally a bad practice. But that is just my opinion which may have something to do with the fact, that I don't need to put SQL statements in code in my line of work, but I do recognize that sometimes there is no other way.
-
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
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). -
While working with a client to help them clean up their code I found this little gem. They absolutely never knew string.Format existed :wtf:
string sql = "select * from table where id={0} and date={1}";
string cmdText = sql.replace("{0}", id.Tostring())
.replace("{1}", DateTime.Now.ToShortDateString());
I know the language. I've read a book. - _Madmatt
String.Format is a nice feature, but slower than just appending.
"I do not know with what weapons World War 3 will be fought, but World War 4 will be fought with sticks and stones." Einstein "Few things are harder to put up with than the annoyance of a good example." Mark Twain
-
String.Format is a nice feature, but slower than just appending.
"I do not know with what weapons World War 3 will be fought, but World War 4 will be fought with sticks and stones." Einstein "Few things are harder to put up with than the annoyance of a good example." Mark Twain
VectorX wrote:
just appending
String concatenation? X| X|
I know the language. I've read a book. - _Madmatt
-
String.Format is a nice feature, but slower than just appending.
"I do not know with what weapons World War 3 will be fought, but World War 4 will be fought with sticks and stones." Einstein "Few things are harder to put up with than the annoyance of a good example." Mark Twain
It's (almost) the right tool for this job.
-
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
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)
{
// Here be dragons ( thousands of lines of totally worthless code)
....
}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.