Stuck with a SQL query.

[Solarion]

I have a list of account numbers

List<string> accountNumbers;

And I'm trying to find all accounts in the database in this range. So something like:

SqlCommand cmd = new SqlCommand();
cmd.CommandText = "SELECT [PaymentID],[Status],[Destination] FROM [Payments].[dbo].[PaymentDetails] where AccountNo in ('"+ accountNumbers + "')";
cmd.CommandType = CommandType.Text;
cmd.Connection = sqlCon;

I know I'm having one of those stupid days and I just can't see it. I know it's wrong but I have no idea why.

 

[dabean]

https://stackoverflow.com/questions/2377506/pass-array-parameter-in-sqlcommand

 

[Johnatan56]

Is account number an int and you're passing a string?

 

[Steamy Tom]

https://stackoverflow.com/questions/4330473/listlong-to-comma-delimited-string-in-c-sharp, i get you have a list of string, but you get the point

 

[Solarion]

Is account number an int and you're passing a string?

Account number is a string.

Thanks for the replies guys I will check them out.

 

[LPCPT]

Every account number must be in quotes and delimited with comma.
Like 'acc1','acc2', etc
It means that you need to traverse the list and add a single quote in front and back of every item.
Then change to
where AccountNo in ("+ accountNumbers + ")"; //remove the single quotes

 

[ozziej]

I would also avoid using string concatenation in a SQL query, that's just asking for a SQL injection attack.

 

[LPCPT]

Just a warning. If these account numbers are entered as input, you are exposed to SQL injection.

 

[GoB]

Unrelated If you were using EF for example it would be:

(from p in ctx.PaymentDetails where accountNumbers.Contains(p.AccountNo) select p)

This would generate a parameterised query (with limitations) rather than dynamic SQL.

*hide*

 

[LPCPT]

I normally use a TVP (table value parameter) to pass a list to a stored procedure.

 

[Kosmik]

Hope your table has an index on the AccountNo field.

 

[Steamy Tom]

i would strongly suspect this is some casual utility/tool and not some production based development that he needs to worry about sql injection, that or they have bigger issues to worry about

 

[Kosmik]

Unrelated If you were using EF for example it would be:

(from p in ctx.PaymentDetails where accountNumbers.Contains(p.AccountNo) select p)

This would generate a parameterised query (with limitations) rather than dynamic SQL.

*hide*

EF

*shudders*

 

[@udiS3]

The SQL injection is a valid concern - user input needs to be sufficiently escaped. PreparedStatement performs this function within Java and .Net has similar API.

Be careful with the amount of account numbers you use within this query. You can bring the dbms to a grinding halt with a large dynamically generated query. What are the criteria for the searched account numbers? Have a look at using a statically typed query, if possible.


Example (This will bring back a list of account numbers for a given person ID no):
select * from payments where account_no in ( select id from account where idnumber = '123456789123');

And paramterize this query:

String sql_acc_query = select * from payments where account_no in ( select id from account where idnumber = ?);
PreparedStatement ps = connection.preparecall(sql_acc_query);
ps.setString("123456789123");

 

[GoB]

EF

*shudders*

Agreed, I realised recently after "Droid" or someone's comments that there is more to EF which I dislike than like.

What I do like having which you could get to in many ways, is being able to effortlessly write queries with compile time static typing against a database.

 

[Solarion]

https://stackoverflow.com/questions/2377506/pass-array-parameter-in-sqlcommand

I went with the solution posted by J Adam Rogers. Seems to be ok and guard against SQL Injection.

https://stackoverflow.com/a/16426999

 

[[)roi(]]

You were pretty close to a solution... still not using *cough* store procedures?

 

[Solarion]

You were pretty close to a solution... still not using *cough* store procedures?

Yeah I know! I'm going to clear this up with the deployment team tomorrow and ask them why they didn't put this in the spec and perhaps should. This is one of those rare rush jobs so I'm sure it's an oversight.

You are spot on there.

 

[[)roi(]]

Agreed, I realised recently after "Droid" or someone's comments that there is more to EF which I dislike than like.

What I do like having which you could get to in many ways, is being able to effortlessly write queries with compile time static typing against a database.

No worries; we all have our preferences.
The Linq syntax in EF, as you probably know, is very similar to the abandoned Linq2SQL i.e. you embed these queries into the C# app as opposed to stored procedures on the DB.

In opposition:
Consider the following; if I needed to build another app for a different platform e.g. using Kotlin instead of C#: I wouldn't lose as much if I had used stored procedures as opposed to Linq2Entities. Similarly I theoretically wouldn't e.g. have to recompile the client app if there was a bug or optimisation issue in a stored procedure. Of course if you building a web api in C#; this wouldn't really matter as much.

 

[XennoX]

A bit late to the party here, the SQL `in` operator is a (relatively speaking) expensive operation - though sometimes it is the only solution.

In the case that the amount of items you need to filter for is large, it may be a better idea to place the numbers to be searched in an in-memory, temporary table or permanent table (watch out for concurrency issues here) and use an inner join to filter the result set.